chore(drive-abci): update to nested address in SML

[shumkov]

Issue being fixed or feature implemented

Bumps all rust-dashcore workspace deps 5c0113e7 → 64d9267d to pick up rust-dashcore#813 — the DashPay friendship-path fix that honors the account index (m/9'/5'/15'/account'/…) instead of hardcoding 0'. That fix is the prerequisite for non-zero / multi-account DashPay contacts in the wallet stack.

Bumping two commits past the current pin also pulls in rpc-json#808 (Core 23 nested masternode platform addresses) — a breaking change to DMNState / DMNStateDiff that required correctly resolving masternode platform ports in drive-abci.

What was done?

• Rev bump of the 8 rust-dashcore deps (dashcore, key-wallet*, dash-spv, dashcore-rpc, …) + Cargo.lock.
• feat: make verification functions public #813 is additive/backward-compatible — no platform-side code change needed.
• fix(ci): fix release workflow syntax error #808 — Core 23 platform-port resolution. Core 23 (DEPLOYMENT_V24, live on devnets) returns ProTx v3 masternodes whose protx listdiff entries zero the deprecated flat platformP2PPort/platformHTTPPort and carry the real ports only in the new nested addresses object. The flat fields were renamed legacy_* and resolver accessors platform_p2p_address() / platform_http_address() added. drive-abci now resolves ports through the accessors (prefer nested addresses, fall back to the non-zero legacy field) everywhere it reads masternode platform ports:
  • Full state — From<DMNState> for MasternodeStateV0 and new_validator_if_masternode_in_state use platform_*_address() (Core 22 maps byte-identically; Core 23 entries are no longer dropped from validator sets). The advertised validator node_ip is taken from the platform p2p host, which is the Core 23 platform host (may differ from the core service IP) and the service IP for a legacy node.
  • Incremental diff — the affected path (update_state_masternode_list_v0): the refresh trigger is diff_triggers_validator_refresh, an intentional superset of the validity predicate validator_refresh_from_state — it fires on a p2p/http port change, any addresses delta (three-state None/Some(None)/Some(Some)), and the zeroed-legacy-port / platform_node_id deltas the port resolvers deliberately drop (a false-positive refresh is idempotent; a false-negative leaves a stale validator advertised). The per-validator update is rebuilt from the post-apply_diff full state via validator_refresh_from_state (single source of truth), and the port resolvers diff_platform_{p2p,http}_port use the nested-first, non-zero legacy fallback so a zeroed v3 legacy port never sets a validator to port 0.
  • Ports disappear — when a diff clears addresses (or zeroes the ports with no addresses) and the updated state resolves no platform ports (or has no platform_node_id), the now-invalid HPMN validator is removed from the validator sets rather than left advertising a stale/dead endpoint.
  • Split-host persistence (MasternodeStateV1) — Core 23 decouples the platform endpoints from the core service address, so the platform host can differ from service.ip(). MasternodeStateV0 stored only the ports, so a normal restart (which loads the masternode list from disk and applies incremental Core diffs) reconstructed the host as service.ip() and advertised the wrong endpoint for a split-host node until the next addresses diff. A new versioned MasternodeStateV1/MasternodeV1/Masternode::V1 persists the platform host, gated to protocol v12 via a new DRIVE_ABCI_STRUCTURE_VERSIONS_V2 (masternode: 1). MasternodeStateV0 stays byte-identical (v1–v11 keep writing V0); the read path is variant-tag driven (no version gate), so any binary decodes either variant. The serialized platform state lives in the non-hashed aux CF (put_aux), so this format change is never part of app_hash — local, non-consensus state that self-heals on the next Core addresses diff.
  • Remaining #[allow(deprecated)] is scoped to the few sites that still name a legacy_* field directly (the two diff-port resolvers, the reverse MasternodeState{V0,V1} → DMNState construction, and test fixtures).

How Has This Been Tested?

• New drive-abci unit tests:
  • Full-state Core-23 resolution + Core-22 fallback (from_dmn_state_*); reverse-conversion clear (reverse_from_reconstructs_core23_addresses_then_honors_a_clear).
  • Diff-port resolution incl. http-only, zeroed-legacy-dropped, out-of-range-dropped (diff_*); validity predicate (refresh_resolves_*, refresh_none_*); and the refresh-trigger superset (refresh_trigger_fires_on_zeroed_legacy_port, refresh_trigger_fires_on_platform_node_id_change, refresh_trigger_none_on_unrelated_diff).
  • Split-host persistence round-trips (v1_round_trip_preserves_distinct_platform_host — host preserved through V1 where V0 collapses it to the service IP; v1_legacy_node_falls_back_to_service_ip; v1_masternode_list_item_round_trip_preserves_host).
  • Backward-compat: the existing real testnet-v0 / devnet-v8 platform-state deserialization fixtures stay green (V0 untouched).
• Also folds in the pre-existing sdk_builder_default_seeds_atomic_to_floor expectation fix (PR fix(sdk): refresh SDK protocol version to the network's on startup and network switch #3886 floor raise, was feature-branch-only on v3.1-dev).
• cargo check --workspace --all-targets clean; clippy -D warnings clean; fmt clean.
• Full CI green (macOS Rust workspace tests, Swift SDK build, JS packages, codecov patch + project).

The split-host design + consensus-safety analysis (4-agent reviewed) is at docs/CORE23_SPLIT_HOST_PERSISTENCE_SPEC.md.

Breaking Changes

None for platform consumers. Masternode platform-port behavior is byte-identical to the current pin for Core 22 (legacy ports). The serialized MasternodeStateV0 format is unchanged; protocol v12 additionally writes a new MasternodeStateV1 that persists the Core 23 platform host — this is local, non-consensus aux-CF state (not in app_hash), readable by any binary that knows the V1 tag, and v1–v11 keep writing V0. Core 23 (v3) entries — which the previous pin mishandled (port 0) — now resolve correctly.

Notes for reviewers

• Supersedes the stale rev bump in feat(swift-sdk): use SPV-synced quorums for Platform proof verification #3417 (which pins an April commit 3f650020, predating the current v3.1-dev pin and not containing feat: make verification functions public #813). Both touch the same Cargo.toml rev line, so they will conflict — this PR should be the canonical dashcore bump.
• Context for the fix(ci): fix release workflow syntax error #808 handling — rust-dashcore#808 documents the devnet (paloma, Core 23.1.3) repro where a state diff's zeroed legacy port corrupted the stored platform port → validator advertised with port 0 → consensus connectivity failure. Mainnet/testnet are latent (V24 NEVER_ACTIVE) until a future Core activation, so the split-host MasternodeStateV1 path is exercised today only on devnets running protocol v12.

Checklist

• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added support for masternode version 1 and introduced drive ABCI structure version 2, enabling Core 23 split platform/core host persistence in v12.
• Bug Fixes
  • Improved masternode and validator endpoint/port resolution, including correct handling when ports are added/removed and resolving Core 23 nested address-based ports vs legacy fields.
  • Refined validator-set cache refresh logic to avoid stale entries.
• Documentation
  • Added a Core 23 split-host masternode persistence specification.
• Chores
  • Updated pinned core library revisions.
• Tests
  • Updated masternode and strategy fixtures for legacy/deprecated port handling and adjusted SDK protocol “floor” seeding assertions.

[coderabbitai]

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title 'chore(drive-abci): update to nested address in SML' is concise and directly references the main change: updating code to handle Core 23's nested address structure in the State Masternode List logic.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/bump-rust-dashcore-dashpay-acct-index

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[thepastaclaw]

✅ Review complete (commit 7641204)

[codecov]

Codecov Report

❌ Patch coverage is 97.60837% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.20%. Comparing base (83fc8c3) to head (7641204).
⚠️ Report is 8 commits behind head on v3.1-dev.

Files with missing lines	Patch %	Lines
...drive-abci/src/platform_types/masternode/v1/mod.rs	94.84%	13 Missing ⚠️
...ernode_list/update_state_masternode_list/v0/mod.rs	99.07%	2 Missing ⚠️
...rs-drive-abci/src/platform_types/masternode/mod.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##           v3.1-dev    #3936       +/-   ##
=============================================
+ Coverage     72.78%   87.20%   +14.41%     
=============================================
  Files            22     2633     +2611     
  Lines          3054   328155   +325101     
=============================================
+ Hits           2223   286152   +283929     
- Misses          831    42003    +41172     

Components	Coverage Δ
dpp	87.70% <ø> (∅)
drive	86.14% <ø> (∅)
drive-abci	89.48% <97.60%> (∅)
sdk	∅ <ø> (∅)
dapi-client	∅ <ø> (∅)
platform-version	∅ <ø> (∅)
platform-value	92.20% <ø> (∅)
platform-wallet	∅ <ø> (∅)
drive-proof-verifier	49.55% <ø> (∅)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/rs-drive-abci/tests/strategy_tests/masternodes.rs`:
- Around line 539-545: The test fixture code increments port values using plain
addition operators on legacy_platform_p2p_port and legacy_platform_http_port,
which can overflow when many updates are applied to the same masternode. Replace
the `*port += 1` operations with saturating or checked arithmetic to prevent
overflow and maintain fixture stability across larger test ranges. Use
saturating_add(1) to safely cap the port values at their maximum rather than
allowing wrapping or panicking.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6cb0cc74-e28a-4349-94c1-3296986b9180

📥 Commits

Reviewing files that changed from the base of the PR and between e1ce320 and 6117689.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (12)

• Cargo.toml
• packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_operator_identity/v0/mod.rs
• packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v0/mod.rs
• packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_voter_identity/v0/mod.rs
• packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/get_operator_identifier/v0/mod.rs
• packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/update_operator_identity/v0/mod.rs
• packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs
• packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs
• packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs
• packages/rs-drive-abci/src/platform_types/validator/v0/mod.rs
• packages/rs-drive-abci/tests/strategy_tests/masternode_list_item_helpers.rs
• packages/rs-drive-abci/tests/strategy_tests/masternodes.rs

[thepastaclaw]

Code Review

Dependency bump pulls in Core 23 DMNState/DMNStateDiff changes where platform ports may live only in a nested addresses field (legacy fields = None) or carry sentinel 0 legacy ports. drive-abci consumes the deprecated raw legacy_platform_*_port fields and ignores addresses, which the upstream test suite at the pinned commit explicitly demonstrates is unsafe: HPMNs are silently dropped from validator sets or assigned port 0. The deferral noted in the PR description is not behavior-preserving against Core 23 RPC output and needs the new accessors (platform_p2p_address() / platform_http_address()) wired in before merge, not after.

🔴 3 blocking | 💬 2 nitpick(s)

2 additional finding(s) omitted (not in diff).

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/platform_types/validator/v0/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/platform_types/validator/v0/mod.rs:28-53: Core 23 HPMNs with nested-only addresses are silently dropped from validator sets
  `new_validator_if_masternode_in_state` destructures only `legacy_platform_p2p_port` / `legacy_platform_http_port` and early-returns `None` if either is absent. The bumped rust-dashcore (rev `64d9267`) documents in `rpc-json/src/lib.rs` (test `dmn_state_core23_addresses_resolve_platform_ports`, ~L3606) that a Core 23 HPMN deserializes with both `legacy_platform_*_port = None` and the real ports under `addresses.platform_p2p` / `addresses.platform_https`. Combined with `try_from_quorum_info_result` filter-mapping `None`, any Core 23 v3 HPMN that registers with only nested addresses disappears from the platform's quorum view — a consensus-critical state divergence the moment any quorum member is in the new format. This is exactly what the upstream accessors `DMNState::platform_p2p_address()` / `platform_http_address()` exist to prevent. The PR description scopes nested-address adoption to a follow-up, but the upstream change is breaking semantics, not syntax: the deferral is not safe. Either consume the accessors here (and store the resolved host/port pair, since the nested `platform_p2p` can publish a host different from `service.ip()`), or hard-gate the rev bump until the consumer migration lands.

In `packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs:140-220: `DMNState` ↔ `MasternodeStateV0` conversion silently drops Core 23 nested addresses
  The forward conversion destructures `addresses: _` and the reverse rebuilds `addresses: None`, so any masternode whose platform ports arrived under the Core 23 `addresses` object is persisted into `MasternodeStateV0` with `platform_p2p_port = None` / `platform_http_port = None`. After a restart or any later refresh, the masternode is then treated by `new_validator_if_masternode_in_state` as missing platform ports and excluded from validator sets. The on-disk `MasternodeStateV0` layout does not have to change to fix this — `From<DMNState>` just needs to fall back to `value.platform_p2p_address()` / `value.platform_http_address()` when the legacy fields are `None`, and (because nested addresses can resolve to a non-service host) the conversion should store the resolved port, ideally with a note that the host pairing is delegated to the validator path. Without this fallback, the PR pins a rust-dashcore that demonstrably loses platform-port information for Core 23 entries (see upstream test `dmn_state_core23_addresses_resolve_platform_ports`).

In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:57-160: Core 23 address-only diffs neither refresh validator entries nor propagate port updates
  Two related problems in this file:

  1. The refresh trigger at lines 150–152 only fires on `pose_ban_height || service || legacy_platform_p2p_port.is_some()`. The upstream test `dmn_state_diff_core23_addresses_resolve_platform_ports` (rpc-json/src/lib.rs ~L3641) shows that an `updatedMNs` entry whose only change is the nested `addresses` deserializes with both `legacy_platform_*_port = None`, so the trigger never fires and validator entries keep stale endpoints after a Core 23 platform port/IP change.

  2. `update_masternode_in_validator_sets` reads `dmn_state_diff.legacy_platform_p2p_port` / `legacy_platform_http_port` and writes `port as u16` straight into the validator. The transitional case in the upstream test `dmn_state_zero_legacy_port_resolves_to_addresses` (rpc-json/src/lib.rs ~L3699) confirms Core 23 can emit `platformP2PPort: 0` alongside real ports in `addresses`. That sentinel currently flows through as `validator.platform_p2p_port = 0`, advertised verbatim to Tenderdash as `tcp://...:0` — a connectivity break the operator did not request. The `as u16` cast also silently truncates ports outside `u16` (upstream test `dmn_state_zero_legacy_port_no_addresses_resolves_to_none` and the 70000-port fixture demonstrate the explicit upstream contract is to reject these).

  Fix should consume `DMNStateDiff::platform_p2p_address()` / `platform_http_address()` (and equivalents on `DMNState`) and gate the refresh trigger on `addresses.is_some()` as well, so v3 ProTx churn cannot inject a port-0 endpoint or skip a needed refresh.

[thepastaclaw]

Code Review

Cumulative review at 3530df8. The PR successfully resolves prior blocking findings on validator/masternode conversion: nested-only Core 23 platform ports are now resolved via DMNState::platform_p2p_address()/platform_http_address() accessors with covering unit tests, and the prior nitpicks on comment direction and saturating_add are also fixed. The masternode-list diff path remains the one carried-forward blocking gap: address-only Core 23 diffs still do not refresh cached validator_sets, and the new inline rationale is factually wrong — DMNStateDiff at rev 64d9267 does expose nested-address accessors (rpc-json/src/lib.rs:2303,2316). The validator host being sourced from service.ip() rather than the nested platform host is recorded as an out-of-scope follow-up since it is pre-existing behavior the PR explicitly defers.

🔴 1 blocking

1 additional finding(s) omitted (not in diff).

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:85-162: Carried-forward prior #3 STILL VALID: Core 23 address-only diffs do not refresh validator_sets; deferral rationale is factually wrong
  Two issues compound on this code path at the bumped rev:

  1) The refresh trigger at lines 152-154 only fires on pose_ban_height/service/legacy_platform_p2p_port. With rust-dashcore 64d9267, DMNStateDiff carries Core 23 platform endpoint changes in `addresses: Option<Option<MasternodeAddresses>>`, and apply_diff propagates them into hpmn_masternode_list. A Core 23 HPMN whose only changed field is the nested platform address is therefore applied to state but never reaches update_masternode_in_validator_sets, so cached ValidatorV0.node_ip/platform_*_port stay stale until an unrelated legacy field changes or the next quorum rotation rebuilds from state. This is deterministic across nodes (no consensus split) but causes bounded liveness lag for cross-validator P2P after Core 23 port-only changes — and contradicts the PR's stated goal of resolving Core 23 nested platform ports in masternode conversion.

  2) The new inline comment at lines 85-88 justifies the deferral by asserting "the diff type has no nested-address accessor." That is false at the pinned rev: rust-dashcore rpc-json/src/lib.rs defines DMNStateDiff::platform_p2p_address() at line 2303 and DMNStateDiff::platform_http_address() at line 2316, mirroring the DMNState accessors this PR already adopts in validator/v0 and masternode/v0. Future maintainers will read this as upstream-not-yet-ready, when in reality the accessors exist and the deferral is a scoping choice.

  Resolution options: (a) extend the trigger to also fire on state_diff.platform_p2p_address().is_some() / platform_http_address().is_some() and consume them inside update_masternode_in_validator_sets; or (b) keep the deferral but rewrite the comment to honestly state that the accessor exists but adoption is deferred to a follow-up, so the rationale is not load-bearing on a false claim.

[thepastaclaw]

Code Review

Carried-forward prior finding #1 is FIXED at d119dd1: the refresh trigger now includes state_diff.platform_p2p_address().is_some() and update_masternode_in_validator_sets reads via the DMNStateDiff::platform_p2p_address()/platform_http_address() accessors with legacy fallback. The codex agents re-frame the same code path as still-valid blockers around host discard and addresses-cleared diffs, but those concerns are explicitly deferred by this PR's scope (MasternodeStateV0 only stores the legacy port pair, host pairing is delegated to the validator path per the documented comment) and they are behavior-preserving relative to Core 22 — they do not break consensus, are not regressions introduced by this PR, and adopting nested platform hosts requires a versioned state migration that is out of scope for a dependency bump. One minor latest-delta suggestion remains: the refresh trigger only checks the p2p axis, so a Core 23 diff that updates only platform_https or carries Some(None) addresses won't fire validator refresh.

🟡 1 suggestion(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:162-166: Validator refresh trigger ignores http-only and cleared-addresses diffs
  The refresh predicate at lines 162-166 only consults the p2p axis (state_diff.platform_p2p_address().is_some() and state_diff.legacy_platform_p2p_port.is_some()). With the bumped rust-dashcore 64d9267, DMNStateDiff::addresses is three-state (None / Some(None) / Some(Some(_))): a Core 23 diff that updates only platform_https (empty platform_p2p), or that clears addresses entirely (Some(None)), leaves platform_p2p_address() returning None and the trigger never fires — so the cached validator entry keeps a stale platform_http_port and/or stale p2p port until the next pose-ban/service/p2p change. In practice today's Core 23 RPC ships p2p and https together so the dominant case is covered, but the cleanest fix is to also check the http accessor, the legacy http field, and the raw addresses presence so any addresses-bearing diff refreshes the validator entry. Validator_sets is local node cache (not consensus state), so impact is wrong-port routing/liveness rather than consensus divergence, hence suggestion-level.

[thepastaclaw]

Code Review

The current head is 1199b87, and the prior refresh-trigger issue is fixed by the new http/legacy/raw-address predicate. One in-scope validator-cache correctness issue remains: address-clearing or empty Core 23 address diffs update the stored HPMN state but leave existing cached validator ports unchanged.

🔴 1 blocking

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:71-77: Clear cached validators when platform ports disappear
  The new refresh predicate now reaches this helper for any `state_diff.addresses.is_some()`, but the helper only writes ports when `diff_platform_p2p_port` or `diff_platform_http_port` returns `Some`. For Core 23 diffs such as `addresses: Some(None)` or an `addresses` object whose platform port list is empty/invalid, upstream `DMNState::apply_diff` removes the resolvable platform address from the already-updated HPMN state, and `ValidatorV0::new_validator_if_masternode_in_state` would return `None` because both platform ports are mandatory. The existing cached `ValidatorV0` in each validator set is still retained with its old `platform_p2p_port` and `platform_http_port`, so Tenderdash can continue receiving stale endpoint data that no longer matches Core state. Rebuild the validator entry from the updated HPMN state, or remove the cached member when the updated state no longer resolves the required platform ports.

[thepastaclaw]

Coverage note for my review at 1199b87: Claude reviewers and the Claude verifier could not run because the Claude ACP account hit its usage limit (resets 8am America/Chicago). Codex general, security-auditor, rust-quality, and Codex verifier ran successfully; CodeRabbit had 0 inline findings and was rate-limited.

[thepastaclaw]

Code Review

Prior 1199b87 finding (stale cached validators when platform ports vanish) is resolved on this head by the new resolves_platform_validator_ports predicate that branches between update and remove. Remaining in-scope concerns: the new accessor-based code throws away the host string returned by platform_p2p_address()/platform_http_address(), and the persistence roundtrip collapses Core 23 addresses into legacy ports in a way that can mask a later addresses-clear diff after restart. No new blocking defects.

🟡 2 suggestion(s) | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/platform_types/validator/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/platform_types/validator/v0/mod.rs:30-36: Validator construction discards the platform host returned by the Core 23 accessor
  `platform_p2p_address()` / `platform_http_address()` return `(host, port)`, but both the validator constructor here and `update_masternode_in_validator_sets` (update_state_masternode_list/v0/mod.rs:73-79) bind `_host` and copy only the port, then use `state.service.ip()` for `node_ip`. `ValidatorSetV0::to_update` advertises Tenderdash endpoints as `node_id@node_ip:platform_p2p_port`, so if a Core 23 ProTx v3 entry ever carries a platform host that differs from the Core service host, Platform will advertise the validator on the wrong host. The pre-existing convention has been service.ip() == platform host, so this likely does not fire in current devnet deployments — but now that the accessor explicitly surfaces a host, please either propagate it into `node_ip` (and refresh on `addresses` diffs) or document why the service IP is intentionally authoritative over the Core 23 platform host.

In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:149-165: Persisted-state roundtrip can mask a Core 23 addresses-clear diff after restart
  `MasternodeStateV0 -> DMNState` (packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs:217-219) reconstructs persisted Core 23 state as `addresses: None` with the resolved port written into `legacy_platform_p2p_port`/`legacy_platform_http_port`. After a restart, a subsequent Core 23 diff with `addresses: Some(None)` (explicit clear) and no legacy-port update applies as `addresses: Some(None)` + `legacy_*: Some(<old>)`. `state.platform_p2p_address()` then falls back to the synthesized legacy port, so `resolves_platform_validator_ports` returns true and the cached validator is retained even though Core has cleared the platform endpoint. Either treat an explicit `state_diff.addresses` delta as authoritative for the validity check (`Some(Some(_))` → check diff accessors; `Some(None)` → false; `None` → full-state fallback), or change the persistence shape to remember address provenance.

[thepastaclaw]

Code Review

Latest delta resolves all three prior 8e19e2a findings (validator node_ip now propagates Core 23 platform host; persisted-state round-trip reconstructs addresses with legacy fields cleared so a Some(None) clear is honored; refresh predicate replaced by validator_refresh_from_state that mirrors the full validity gate including platform_node_id). Two narrow gaps remain in the refresh-trigger ↔ refresh-predicate symmetry (zeroed legacy ports and platform_node_id deltas), plus the persisted-state path still loses the Core 23 platform host across restart. None are exploitable in current Core 23 deployments (collocated hosts; Core does not emit standalone zero-port diffs), so they are suggestions rather than blockers.

🟡 2 suggestion(s) | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:141-146: Refresh trigger misses zeroed/legacy-only platform-port deltas the predicate would drop
  The removal branch in apply_diff fires only when `diff_platform_p2p_port`/`diff_platform_http_port` return Some or addresses is in the diff. Both helpers intentionally filter `0` and out-of-range legacy ports (line 211/222). The code comment at lines 155–159 explicitly lists "a zeroed legacy port with no addresses" as one of the removal cases, but a state_diff carrying only `legacy_platform_p2p_port: Some(0)` (or an out-of-range value) with no addresses delta and no service/pose_ban change does not trip the guard, so `validator_refresh_from_state` is never consulted and the stale validator entry stays advertised. In practice Core 23 diffs that zero the legacy field also carry an `addresses` delta, but the trigger should not depend on that — the refresh predicate is the source of truth and the trigger should mirror its full gate. Same class of inconsistency this PR is paying down elsewhere. Easiest fix: include `state_diff.legacy_platform_p2p_port.is_some() || state_diff.legacy_platform_http_port.is_some()` (and optionally `state_diff.platform_node_id.is_some()`) in the guard so any field that can change validity wakes up the refresh.

In `packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs:211-224: Persisted Core 23 state loses the platform host across restart
  `MasternodeStateV0` stores only the resolved platform ports, so this reverse conversion synthesizes `addresses` using `service.ip()` as the host. The live validator path (validator/v0/mod.rs:42 and validator_refresh_from_state at update_state_masternode_list/v0/mod.rs:244) now treats the Core 23 `addresses.platform_p2p` host as authoritative — the PR's own test `refresh_resolves_core23_addresses_with_platform_host` (line 385) deliberately uses a platform host distinct from the service IP. After a Platform restart, any Core 23 HPMN whose platform host differs from its core service IP is reconstructed with the wrong host; subsequent quorum/validator-set construction will advertise `node_id@service_ip:platform_p2p_port` to Tenderdash until the next `addresses` diff overwrites it. Today's deployments collocate the two hosts so this is latent, but the PR ships and tests the distinct-host path, leaving the persistence layer inconsistent with the live path's claimed support. Either persist the platform host in `MasternodeStateV0` (versioned schema change) or document the post-restart re-converge behavior and the collocated-host invariant.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/rs-drive-abci/src/platform_types/masternode/v1/mod.rs`:
- Around line 155-158: The platform_host derivation at line 157 only extracts
the host from platform_p2p_address(), which causes data loss when only
platform_http_address() resolves. Refactor the platform_host extraction to use a
fallback pattern: first attempt to extract the host from platform_p2p (the
current approach), and if that yields None, then extract the host from
platform_http_address() as a fallback. This ensures the host information is
preserved regardless of whether the P2P or HTTP address is the one that
resolves, maintaining a lossless round-trip conversion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fec25ade-3134-4113-9314-1758540cc1a7

📥 Commits

Reviewing files that changed from the base of the PR and between 369f563 and a05da4d.

📒 Files selected for processing (8)

• docs/CORE23_SPLIT_HOST_PERSISTENCE_SPEC.md
• packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs
• packages/rs-drive-abci/src/platform_types/masternode/accessors.rs
• packages/rs-drive-abci/src/platform_types/masternode/mod.rs
• packages/rs-drive-abci/src/platform_types/masternode/v1/mod.rs
• packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_structure_versions/mod.rs
• packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_structure_versions/v2.rs
• packages/rs-platform-version/src/version/v12.rs

✅ Files skipped from review due to trivial changes (2)

• packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_structure_versions/v2.rs
• packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_structure_versions/mod.rs

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

• packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs

[thepastaclaw]

Code Review

All three prior findings are resolved by the latest delta: validator-refresh trigger now includes zeroed legacy ports + platform_node_id deltas, and MasternodeStateV1 persists the Core 23 platform host. However, the platform_node_id portion of the trigger fix is incomplete — the refresh struct and write path were not extended to carry node_id, so a node-id rotation takes the refresh path but leaves validator.node_id stale. One additional suggestion: the new Core 23 reverse conversions emit unbracketed IPv6 host:port strings that the upstream parser rejects, breaking the persisted round-trip for IPv6 HPMN nodes.

🟡 1 suggestion(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/platform_types/masternode/v1/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/platform_types/masternode/v1/mod.rs:221-234: Reverse conversion emits unparsable IPv6 platform addresses
  The new Core-23-shaped reverse conversions in v1/mod.rs (line 221-234) and v0/mod.rs (line 211-224) rebuild platform_p2p / platform_https entries via `format!("{host}:{port}")` where host is either `service.ip()` (an IpAddr) or `platform_host` (a String that for legacy nodes is captured from `service.ip().to_string()`). For an IPv6 service such as `2001:db8::1`, IpAddr's Display impl produces the bare unbracketed form, so the stored string becomes `2001:db8::1:36656`. The upstream nested-address parser rejects unbracketed multi-colon hosts, so DMNState::platform_p2p_address() returns None after a save/load round-trip and new_validator_if_masternode_in_state drops the HPMN from validator sets until a later Core addresses diff. This breaks the persisted round-trip exactly for IPv6 HPMN operators. Fix by routing IPv6 hosts through SocketAddr::new(ip, port).to_string() (which brackets correctly) when the host is an IpAddr, or by wrapping any String host containing ':' but not already starting with '[' in brackets before formatting. The same fix should be applied to v0/mod.rs lines 211-224.

[shumkov]

Addressed all open review threads in a05da4d36c … 2979a26613 — see inline replies. Summary: split-host persistence (MasternodeStateV1, protocol-v12-gated, non-consensus aux-CF state), refresh-trigger completeness (diff_triggers_validator_refresh superset), IPv6 platform-host bracketing, and the validator-reconcile refactor (now a function of state). Design + 4-agent consensus-safety analysis: docs/CORE23_SPLIT_HOST_PERSISTENCE_SPEC.md.

[thepastaclaw]

Code Review

Reconciled two prior findings against head 073d68c — both remain STILL VALID. The latest delta refactored validator reconciliation to derive from full post-apply_diff state, which is semantically equivalent and a clean cleanup, but did NOT touch (1) the ValidatorRefresh field set (still missing platform_node_id) nor (2) the IPv6 host:port formatting in v0/v1 reverse conversions. claude-general also surfaces a real but lower-impact V0 round-trip issue that masks Core-22 legacy port changes after restart by eagerly synthesizing an addresses field; kept as a suggestion.

🔴 1 blocking | 🟡 1 suggestion(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs:211-224: Reverse conversions emit unparsable IPv6 platform addresses (v0 always, v1 legacy fallback)
  Both reverse conversions rebuild the Core 23 nested `addresses` entries via `format!("{host}:{port}")`. In v0/mod.rs:211, `host = service.ip()` is an `IpAddr`; for IPv6 its `Display` impl produces a bare unbracketed string, so the persisted entry becomes e.g. `2001:db8::1:36656`. The bumped rust-dashcore `parse_host_port` (rpc-json/src/lib.rs:2088-2098) explicitly rejects unbracketed multi-colon hosts: `if host.contains(':') && !(host.starts_with('[') && host.ends_with(']')) { return None; }` — verified by the `parse_host_port_ipv6` test. After a save/load round-trip on an IPv6 HPMN, `DMNState::platform_p2p_address()` returns `None` and `new_validator_if_masternode_in_state` drops that HPMN from validator sets until a later Core addresses diff (which Core 22 never emits). v1/mod.rs:221 has the same defect on the fallback path: `host = platform_host.unwrap_or_else(|| service.ip().to_string())` — for a Core 22 legacy IPv6 node the captured `platform_host` itself comes from `legacy_platform_address` (rust-dashcore lib.rs:2174) which uses `self.service.ip().to_string()` (unbracketed). Note: the Core 23 nested-addresses path for v1 is safe because `first_valid_host_port` retains the brackets it parsed in. Fix by wrapping IPv6 hosts in brackets before formatting, e.g. `SocketAddr::new(ip, port).to_string()` when starting from an `IpAddr`, or bracketing a `String` host that contains `:` and is not already bracketed.
- [SUGGESTION] packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs:184-243: V0 reverse synthesizes `addresses` from a Core-22 port, masking subsequent legacy port-change diffs after restart
  `From<MasternodeStateV0> for DMNState` unconditionally builds `addresses = Some(Some(MasternodeAddresses{ ..host:stored_port.. }))` whenever either port is present, and leaves `legacy_platform_*_port = None`. That is the right shape for a Core-23 origin (so an `addresses: Some(None)` clear diff can drop the endpoint, per the `reverse_from_reconstructs_core23_addresses_then_honors_a_clear` test). But it is also applied to persisted state that originally came from a Core-22 node — i.e. every HPMN on mainnet/testnet today, where the protocol version is pre-v12 and V0 is the only persistence variant in use. After a drive-abci restart on Core 22: the reloaded DMNState has `addresses = Some(<host:old_port>)` and `legacy_*_port = None`; the next Core-22 diff carries `legacy_platform_p2p_port = Some(Some(new_port))` with `addresses = None`; after `apply_diff` the state has `addresses = Some(<host:old_port>)` AND `legacy_platform_p2p_port = Some(new_port)`. Per `DMNState::platform_p2p_address()` (rust-dashcore lib.rs:2147-2152, which prefers `addresses` over the legacy fallback), the resolver returns the stale host:old_port, and `new_validator_if_masternode_in_state` / `validator_refresh_from_state` advertise the old port until a later addresses-bearing diff — which Core 22 never emits. Pre-PR the platform port was a flat field that the legacy path updated directly. Two reasonable fixes: (a) leave `legacy_*_port = Some(stored_port)` and `addresses = None` in the reverse when there is no captured platform host (V0 has none), since the legacy path remains correct for Core-22 environments and a Core-23 transition would emit an addresses diff anyway; or (b) consult `platform_version` and only synthesize `addresses` when the active protocol expects Core 23.

[thepastaclaw]

🔴 Blocking: Reverse conversions emit unparsable IPv6 platform addresses (v0 always, v1 legacy fallback)

Both reverse conversions rebuild the Core 23 nested addresses entries via format!("{host}:{port}"). In v0/mod.rs:211, host = service.ip() is an IpAddr; for IPv6 its Display impl produces a bare unbracketed string, so the persisted entry becomes e.g. 2001:db8::1:36656. The bumped rust-dashcore parse_host_port (rpc-json/src/lib.rs:2088-2098) explicitly rejects unbracketed multi-colon hosts: if host.contains(':') && !(host.starts_with('[') && host.ends_with(']')) { return None; } — verified by the parse_host_port_ipv6 test. After a save/load round-trip on an IPv6 HPMN, DMNState::platform_p2p_address() returns None and new_validator_if_masternode_in_state drops that HPMN from validator sets until a later Core addresses diff (which Core 22 never emits). v1/mod.rs:221 has the same defect on the fallback path: host = platform_host.unwrap_or_else(|| service.ip().to_string()) — for a Core 22 legacy IPv6 node the captured platform_host itself comes from legacy_platform_address (rust-dashcore lib.rs:2174) which uses self.service.ip().to_string() (unbracketed). Note: the Core 23 nested-addresses path for v1 is safe because first_valid_host_port retains the brackets it parsed in. Fix by wrapping IPv6 hosts in brackets before formatting, e.g. SocketAddr::new(ip, port).to_string() when starting from an IpAddr, or bracketing a String host that contains : and is not already bracketed.

_{source: ['claude', 'codex']}

[thepastaclaw]

Resolved in 2979a26 — Reverse conversions emit unparsable IPv6 platform addresses (v0 always, v1 legacy fallback) no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[thepastaclaw]

🟡 Suggestion: V0 reverse synthesizes addresses from a Core-22 port, masking subsequent legacy port-change diffs after restart

From<MasternodeStateV0> for DMNState unconditionally builds addresses = Some(Some(MasternodeAddresses{ ..host:stored_port.. })) whenever either port is present, and leaves legacy_platform_*_port = None. That is the right shape for a Core-23 origin (so an addresses: Some(None) clear diff can drop the endpoint, per the reverse_from_reconstructs_core23_addresses_then_honors_a_clear test). But it is also applied to persisted state that originally came from a Core-22 node — i.e. every HPMN on mainnet/testnet today, where the protocol version is pre-v12 and V0 is the only persistence variant in use. After a drive-abci restart on Core 22: the reloaded DMNState has addresses = Some(<host:old_port>) and legacy_*_port = None; the next Core-22 diff carries legacy_platform_p2p_port = Some(Some(new_port)) with addresses = None; after apply_diff the state has addresses = Some(<host:old_port>) AND legacy_platform_p2p_port = Some(new_port). Per DMNState::platform_p2p_address() (rust-dashcore lib.rs:2147-2152, which prefers addresses over the legacy fallback), the resolver returns the stale host:old_port, and new_validator_if_masternode_in_state / validator_refresh_from_state advertise the old port until a later addresses-bearing diff — which Core 22 never emits. Pre-PR the platform port was a flat field that the legacy path updated directly. Two reasonable fixes: (a) leave legacy_*_port = Some(stored_port) and addresses = None in the reverse when there is no captured platform host (V0 has none), since the legacy path remains correct for Core-22 environments and a Core-23 transition would emit an addresses diff anyway; or (b) consult platform_version and only synthesize addresses when the active protocol expects Core 23.

_{source: ['claude']}

[thepastaclaw]

Code Review

Cumulative re-validation at HEAD 7641204: Prior #1 (validator refresh ignores rotated platform_node_id) is STILL VALID — ValidatorRefresh (v0/mod.rs:209-214) carries only node_ip/ports/is_banned and validator_refresh_from_state discards state.platform_node_id after gating on it, so a node-id rotation fires the trigger but writes a stale node_id. Prior #2 (IPv6 bracketing in reverse conversions) is FIXED by commit 2979a26 via format_platform_address (masternode/mod.rs:62-68), wired into both V0 and V1 reverse paths and covered by brackets_only_unbracketed_ipv6. Prior #3 (V0 reverse synthesizes nested addresses from a Core-22 port, masking later legacy port diffs after restart) is STILL VALID — From<MasternodeStateV0> for DMNState (v0/mod.rs:184-242) unconditionally builds addresses = Some(..) whenever any platform port is present and clears legacy fields. No genuinely new latest-delta findings beyond the carry-forwards.

🔴 1 blocking

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:62-72: Carried-forward (STILL VALID): validator refresh never writes the rotated platform_node_id (or service core_port)
  Re-validated at HEAD 7641204b. `diff_triggers_validator_refresh` correctly fires on `diff.platform_node_id.is_some()` (line 257) and `diff.service.is_some()` (line 251), and `refresh_trigger_fires_on_platform_node_id_change` asserts the trigger. The write path is incomplete:

  - `ValidatorRefresh` (lines 209-214) only carries `node_ip`, `platform_p2p_port`, `platform_http_port`, `is_banned`.
  - `validator_refresh_from_state` (lines 224-234) gates on `state.platform_node_id?` but discards the value rather than returning it.
  - `refresh_validator_in_sets_from_state` (lines 62-71) rewrites only those same four fields.

  But `ValidatorV0` (packages/rs-dpp/src/core_types/validator/v0/mod.rs) also has `node_id: PubkeyHash` and `core_port: u16`, both initialized from `state.platform_node_id` / `state.service.port()` in `new_validator_if_masternode_in_state` and both used in the Tenderdash-advertised `node_id@host:port` triple.

  Consequences:
  1. A `platform_node_id`-only diff fires the trigger, `validator_refresh_from_state` returns `Some` (because `state.platform_node_id` is still set), and the validator's `node_id` retains its stale pre-rotation value. Tenderdash advertises `old_node_id@host:port` until a full validator-set rebuild — exactly the bug the new trigger was added to prevent.
  2. A Core-service-port change (`DMNStateDiff::service.is_some()`) similarly fires the trigger but the cached `core_port` is never rewritten.

  The existing `refresh_trigger_fires_on_platform_node_id_change` test only asserts the trigger predicate; it does not exercise the write path. Fix shape: include `node_id: PubkeyHash` and `core_port: u16` in `ValidatorRefresh`, populate them in `validator_refresh_from_state` (mirroring `new_validator_if_masternode_in_state`), write them in `refresh_validator_in_sets_from_state`, and add a write-path test asserting the updated `node_id`/`core_port` on the validator entry.

[thepastaclaw]

🔴 Blocking: Carried-forward (STILL VALID): validator refresh never writes the rotated platform_node_id (or service core_port)

Re-validated at HEAD 7641204. diff_triggers_validator_refresh correctly fires on diff.platform_node_id.is_some() (line 257) and diff.service.is_some() (line 251), and refresh_trigger_fires_on_platform_node_id_change asserts the trigger. The write path is incomplete:

• ValidatorRefresh (lines 209-214) only carries node_ip, platform_p2p_port, platform_http_port, is_banned.
• validator_refresh_from_state (lines 224-234) gates on state.platform_node_id? but discards the value rather than returning it.
• refresh_validator_in_sets_from_state (lines 62-71) rewrites only those same four fields.

But ValidatorV0 (packages/rs-dpp/src/core_types/validator/v0/mod.rs) also has node_id: PubkeyHash and core_port: u16, both initialized from state.platform_node_id / state.service.port() in new_validator_if_masternode_in_state and both used in the Tenderdash-advertised node_id@host:port triple.

Consequences:

1. A platform_node_id-only diff fires the trigger, validator_refresh_from_state returns Some (because state.platform_node_id is still set), and the validator's node_id retains its stale pre-rotation value. Tenderdash advertises old_node_id@host:port until a full validator-set rebuild — exactly the bug the new trigger was added to prevent.
2. A Core-service-port change (DMNStateDiff::service.is_some()) similarly fires the trigger but the cached core_port is never rewritten.

The existing refresh_trigger_fires_on_platform_node_id_change test only asserts the trigger predicate; it does not exercise the write path. Fix shape: include node_id: PubkeyHash and core_port: u16 in ValidatorRefresh, populate them in validator_refresh_from_state (mirroring new_validator_if_masternode_in_state), write them in refresh_validator_in_sets_from_state, and add a write-path test asserting the updated node_id/core_port on the validator entry.

_{source: ['claude', 'codex']}