fix: empty platformP2PPort deprecated field in protx listdiff results

[knst]

Issue being fixed or feature implemented

There're 2 different versions of protx's internal storages; and they are switched after v24 activation.

It seems as when version switched, the incorrect platformP2PPort / platformHTTPPort may be added to diff even if they are not used anymore and it happens only if snapshot is used; for example after node restart.

It causes strange result of protxlistdiff A B such as:

"MASTERNODE": {
    {
        "lastPaidHeight": 9204,
        "platformP2PPort": 0, <---- INVALID it should be correct port here
        "platformHTTPPort": 0, <----- INVALID it should be correct port here
        "addresses": {
          "platform_p2p": [
            <VALID>
          ],
          "platform_https": [
            <VALID>
          ]

It happened on devnet on block on 9216

What was done?

Added reset of platformP2PPort and platformHTTPPort.
Added a workaround to show valid results for nodes that already have corrupted database.

How Has This Been Tested?

Sync node with devnet palona ; restart node -> issue is reproduced.
Build a node from this PR, restart node -> issue is not reproduced.

Breaking Changes

N/A

Checklist:

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

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[thepastaclaw]

✅ Review complete (commit 9c74948)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6015fd07-355b-44f1-a330-2415205609d6

📥 Commits

Reviewing files that changed from the base of the PR and between f9a4aeb and 9c74948.

📒 Files selected for processing (1)

• src/evo/dmnstate.cpp

💤 Files with no reviewable changes (1)

• src/evo/dmnstate.cpp

---

Walkthrough

This PR makes two coordinated edits: JSON emission in CDeterministicMNStateDiff::ToJson now prefers platform ports stored in state.netInfo (PLATFORM_P2P / PLATFORM_HTTPS) when available, falling back to legacy scalar fields otherwise; and CSpecialTxProcessor::RebuildListFromBlock clears legacy platformP2PPort and platformHTTPPort scalars to 0 for ProTxVersion >= ExtAddr during service updates so reconstructed state matches netInfo-backed storage.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• UdjinM6
• PastaPastaPasta
• thepastaclaw

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing the deprecated platformP2PPort field appearing incorrectly in protx listdiff results due to v24 storage version migration.
Description check	✅ Passed	The description is directly related to the changeset, explaining the root cause (v24 storage version switch), the problem (incorrect zero values for deprecated fields), the solution (reset fields and add workaround), and testing approach.
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.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

src/evo/dmnstate.cpp (1)

94-94: 💤 Low value

Fix subject-verb agreement in comment.

"nodes that has been" should be "nodes that have been".

📝 Suggested fix

-            // TODO: remove this workaround for nodes that has been incorrectly migrated internal storage in the future
+            // TODO: remove this workaround for nodes that have been incorrectly migrated internal storage in the future

🤖 Prompt for 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.

In `@src/evo/dmnstate.cpp` at line 94, Update the TODO comment near the "remove
this workaround for nodes that has been incorrectly migrated internal storage in
the future" text in dmnstate.cpp to correct the grammar: change "nodes that has
been" to "nodes that have been" so the comment reads "remove this workaround for
nodes that have been incorrectly migrated internal storage in the future".

🤖 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.

Nitpick comments:
In `@src/evo/dmnstate.cpp`:
- Line 94: Update the TODO comment near the "remove this workaround for nodes
that has been incorrectly migrated internal storage in the future" text in
dmnstate.cpp to correct the grammar: change "nodes that has been" to "nodes that
have been" so the comment reads "remove this workaround for nodes that have been
incorrectly migrated internal storage in the future".

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8687e076-0171-484e-af28-9e457c35dcbd

📥 Commits

Reviewing files that changed from the base of the PR and between 317917a and f9a4aeb.

📒 Files selected for processing (2)

• src/evo/dmnstate.cpp
• src/evo/specialtxman.cpp

[thepastaclaw]

Code Review

Two-commit fix: specialtxman.cpp clears legacy platformP2PPort/HTTPPort scalars on ExtAddr ProUpServTx (proper forward fix), and dmnstate.cpp renders the port from netInfo when present in the diff (acknowledged partial workaround for already-corrupted databases). The forward fix is sound. The workaround correctly handles diffs that carry netInfo, but cannot recover phantom scalar-only diffs (where the two compared states differ only in the now-legacy scalar). The commit message already acknowledges the workaround is temporary, so this is a known limitation rather than a regression.

🟡 1 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 `src/evo/dmnstate.cpp`:
- [SUGGESTION] src/evo/dmnstate.cpp:95-100: Workaround can't recover phantom scalar-only diffs
  The fallback derives the port from netInfo only when the same CDeterministicMNStateDiff also carries Field_netInfo. For already-corrupted databases, BuildDiff can still produce a scalar-only diff: ExtAddr full-state serialization in CDeterministicMNState (dmnstate.h:105) omits platformP2PPort/HTTPPort, so a snapshot round-trip zeros those members, while older replayed diffs may still bump them to a stale N. If `protx listdiff` straddles such a snapshot boundary and netInfo is unchanged between the two endpoints, the diff carries Field_platformP2PPort/HTTPPort without Field_netInfo, falls through to `state.platformP2PPort`, and reports the phantom value (0 or stale N depending on direction). The commit message already frames this as a workaround that "may be reverted in the future", so this is a known partial fix rather than a regression — flagging because the workaround's stated goal isn't fully met. A complete fix would either suppress these phantom scalar fields in BuildDiff for ExtAddr states, or normalize the rendered port against the full target state's netInfo.

[thepastaclaw]

🟡 Suggestion: Workaround can't recover phantom scalar-only diffs

The fallback derives the port from netInfo only when the same CDeterministicMNStateDiff also carries Field_netInfo. For already-corrupted databases, BuildDiff can still produce a scalar-only diff: ExtAddr full-state serialization in CDeterministicMNState (dmnstate.h:105) omits platformP2PPort/HTTPPort, so a snapshot round-trip zeros those members, while older replayed diffs may still bump them to a stale N. If protx listdiff straddles such a snapshot boundary and netInfo is unchanged between the two endpoints, the diff carries Field_platformP2PPort/HTTPPort without Field_netInfo, falls through to state.platformP2PPort, and reports the phantom value (0 or stale N depending on direction). The commit message already frames this as a workaround that "may be reverted in the future", so this is a known partial fix rather than a regression — flagging because the workaround's stated goal isn't fully met. A complete fix would either suppress these phantom scalar fields in BuildDiff for ExtAddr states, or normalize the rendered port against the full target state's netInfo.

_{source: ['codex']}

[thepastaclaw]

Correction: this prior finding was carried forward, not resolved. The latest review for 9c74948 keeps the same scalar-only ExtAddr diff concern as a suggestion under the updated title “Workaround still misses scalar-only ExtAddr diffs”.

_{Edited to correct the automated reconciliation note.}

[thepastaclaw]

Code Review

The latest delta only removes the malformed TODO comment, so there are no new findings in 9c74948. The prior TODO grammar finding is resolved, but the prior scalar-only corrupted-database gap remains in scope because the PR explicitly claims to show valid results for nodes that already have corrupted internal storage.

🟡 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 `src/evo/dmnstate.cpp`:
- [SUGGESTION] src/evo/dmnstate.cpp:94-105: Workaround still misses scalar-only ExtAddr diffs
  The fallback only derives deprecated platform ports from `state.netInfo` when this same `CDeterministicMNStateDiff` carries `Field_netInfo`. That does not cover an already-corrupted database where an ExtAddr full-state snapshot round-trip has zeroed the legacy scalar fields, while a diff-reconstructed list still has stale scalar values and unchanged `netInfo`. `CDeterministicMNStateDiff` compares `platformP2PPort` and `platformHTTPPort` independently, so that case can produce a scalar-only diff; then `has_ext_netinfo` is false and `ToJson()` falls back to the stale scalar values. The forward fix in `specialtxman.cpp` prevents newly written stale scalars, but this workaround still does not satisfy the PR's stated goal of valid `protx listdiff` results for nodes with already-corrupted storage.

[thepastaclaw]

🟡 Suggestion: Workaround still misses scalar-only ExtAddr diffs

The fallback only derives deprecated platform ports from state.netInfo when this same CDeterministicMNStateDiff carries Field_netInfo. That does not cover an already-corrupted database where an ExtAddr full-state snapshot round-trip has zeroed the legacy scalar fields, while a diff-reconstructed list still has stale scalar values and unchanged netInfo. CDeterministicMNStateDiff compares platformP2PPort and platformHTTPPort independently, so that case can produce a scalar-only diff; then has_ext_netinfo is false and ToJson() falls back to the stale scalar values. The forward fix in specialtxman.cpp prevents newly written stale scalars, but this workaround still does not satisfy the PR's stated goal of valid protx listdiff results for nodes with already-corrupted storage.

_{source: ['codex']}

[DashCoreAutoGuix]

Guix Automation has began to build this PR tagged as v23.1.3-devpr7360.9c749489. A new comment will be made when the image is pushed.

[DashCoreAutoGuix]

Guix Automation has failed with an unknown error for tag v23.1.3-devpr7360.9c749489

[PastaPastaPasta]

utACK 9c74948