[shumkov]

Issue being fixed or feature implemented

#5629 (shipped in 5.0.2, forward-merged into v5.1-dev) made prepare_masternode_update_service refuse a legacy evonode's V24 conversion when Dash Core v24 would refuse the resulting version-3 address map. It did this with a copy of Core's rules:

• IsBadPort (ports 1–1023 plus Core's list of well-known ports) and port 0, with 443 allowed only for Platform HTTPS;
• a map that lists one address twice.

That PR noted the copy would be replaced in v5.1 by rust-dashcore's ExtNetInfo::validate_ports_and_duplicates (dashpay/rust-dashcore#1226). v5.1-dev's pin has carried that helper since #5729, so this PR makes the replacement.

What was done?

In packages/rs-platform-wallet/src/masternode/update_service.rs:

• The conversion builds the address map first, then checks it with ExtNetInfo::validate_ports_and_duplicates, in the new require_core_accepts_converted_net_info.
• Removed the copied CORE_PRIVILEGED_PORTS_THRESHOLD / CORE_BAD_PORTS constants and the wallet's own require_port_core_accepts / require_distinct_ports checks.
• The helper's ExtNetInfoError doesn't say which entry it refuses (BadPort(u16), Duplicate). A converted map has one entry per purpose, all at the legacy IP, so the wallet finds the entry by its port and keeps its two existing messages word for word:
  • Dash Core v24 refuses {purpose} port {port}, … move the service to another port and register it with Dash Core's protx update_service_evo
  • {other} and {purpose} would both use port {port}, … give each service its own port
• ExtNetInfoError is #[non_exhaustive]. A future variant, or a lookup that finds nothing, falls back to a refusal that includes the helper's error. It is never accepted.

Behaviour: the set of conversions accepted and refused doesn't change. The map holds only IPv4/IPv6 service entries (never I2P or domain entries, never two entries for one purpose), and for that shape the helper applies the same rules the copy did. One difference: if a conversion breaks both rules at once (for example Core P2P and Platform P2P both on port 1), the duplicate is now reported first, because that is the order Core applies them in. Before, the bad port was reported first. The conversion is refused in both cases.

The error variant is still PlatformWalletError::InvalidParameter, so the FFI, Swift and Kotlin error mappings don't change.

Version-3 re-assertion and pre-V24 version-2 payloads still skip this check, as in #5629. Core has already accepted the on-chain map, and Core doesn't apply these rules to version-2 payloads.

How Has This Been Tested?

• The two fix(platform-wallet): refuse ports Dash Core v24 refuses before building a version-3 ProUpServTx #5629 tests pass unchanged:
  • should_refuse_a_legacy_evonode_conversion_with_a_port_core_v24_refuses: every system and well-known port, 0, 443 outside Platform HTTPS, the 1/1023 boundaries; 443 and 1024+ accepted; each error names the purpose and port.
  • should_refuse_a_legacy_evonode_conversion_that_repeats_an_address: Platform P2P on the Core P2P or Platform HTTPS port; the error names both services and the port.
• Rules the converted map can't reach (I2P port 0, domain entries, an address repeated within one purpose) aren't wallet tests. They are pinned by the helper's own port_and_duplicate_rules test cases in rust-dashcore.
• Mutation checks through the new path. Each turned the expected test red, and the file was restored afterwards:
  • skipping the helper call fails both tests;
  • naming the purpose of a different port fails the bad-port test (CoreP2P port 1 reported as PlatformP2P port 1);
  • naming a different pair fails the repeated-address test.
• cargo test -p platform-wallet -p platform-wallet-storage -p platform-wallet-ffi --all-features --locked --lib --bins --tests -- --skip wallet::shielded::, with CI's env (RUST_MIN_STACK=4194304, CARGO_PROFILE_DEV_DEBUG=0, CARGO_PROFILE_DEV_CODEGEN_UNITS=256): 3,095 passed, 0 failed (10 tests are #[ignore] on the base).
• CI's scoped clippy (platform-wallet, platform-wallet-storage, platform-wallet-ffi, rs-unified-sdk-ffi, rs-unified-sdk-jni; --all-targets --all-features --locked -- --no-deps -D warnings): clean.
• cargo fmt --check --all: clean.

Breaking Changes

None. This only changes how the wallet builds a transaction; there is no consensus or protocol-version change.

Checklist:

• I have performed a self-review of my own code
• My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request
• If I added or changed a system contract's document schemas, I checked the contract viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · 8333330

• Bots — thepastaclaw ✓
• Self-review — post /self-reviewed
• Reviewer requests paused — your 5 review slots are occupied; this PR is excluded from reviewers' queues. Required approvals still count without a slot.
• Build green
• Approvals
  • rs-platform-wallet (packages/rs-platform-wallet/src/masternode/update_service.rs) — HashEngineering or ZocoLini or llbartekll or romchornyi

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

Summary by CodeRabbit

• Bug Fixes
  • Legacy evonode conversions now apply shared validation rules to generated version-3 address maps and return clearer errors for invalid ports or duplicate services.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Legacy evonode conversion now validates its generated version-3 address map with ExtNetInfo::validate_ports_and_duplicates. It replaces local port and duplicate checks with shared validation and maps validation failures to InvalidParameter errors.

Changes: Wallet address-map validation

Layer / File(s)	Summary
Delegate conversion checks to shared validation packages/rs-platform-wallet/src/masternode/update_service.rs	The conversion uses ExtNetInfo::validate_ports_and_duplicates instead of local port and duplicate checks. It reports the purpose and port for invalid ports, identifies both purposes for duplicate ports, and uses a fallback InvalidParameter error for other validation failures. The conversion documentation now references the shared validation helper.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related PRs

• dashpay/platform#5629: Adds port rules and duplicate-port checks to the same wallet conversion logic that this PR delegates to shared validation.
• dashpay/platform#5746: Includes the wallet port-validation change from #5629 in a merge that forms part of the context for this PR.

Suggested reviewers: quantumexplorer, lklimek

Merge Risk: Minimal · up to 83333

The shared validator preserves conversion acceptance behavior, with duplicate errors reported first. No merge-blocking issue is established.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: the wallet's converted evonode address map is now checked with rust-dashcore's Core port and duplicate rules. The raw summary and objectives both describe replacing th…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: the wallet's converted evonode address map is now checked with rust-dashcore's Core port and duplicate rules. The raw summary and objectives both describe replacing the copied Dash Core v24 checks with ExtNetInfo::validate_ports_and_duplicates. The title is 96 characters, which is long but still readable, and it uses the 'refactor(...)' prefix, which fits the change.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 1 files.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: shumkov/v51-t16-wallet-port-helper
• File: .coderabbit.yaml
• Commit: 8333330
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 474528f4-bdf6-4bdc-b98b-18bd0f15f82e

Commits

Reviewing files that changed from the base of the PR and between e0f21d7 and 8333330.

Files selected for processing: 1

• packages/rs-platform-wallet/src/masternode/update_service.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

✅ Final review complete — no blockers (commit 8333330) · triage: low

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Verified the patch at 8333330 against the pinned rust-dashcore implementation: it preserves acceptance and rejection for converted maps, delegates validation to its owner, and safely retains wallet-specific diagnostics. No actionable defects remain; the proposed fallback test exercises an unreachable input shape, and richer upstream errors are an optional API enhancement. This was a static review; current CI reports wallet tests and Swift/Kotlin checks passing, with review and hygiene checks pending, while the earlier policy failure was an unrelated Python-download HTTP 504 and subsequent runs passed.

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — This is a contained single-file refactor replacing duplicated address-map validation with an existing Core helper while preserving rejection behavior and diagnostic messages apart from error precedence.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — rust-quality (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 100% left, weekly 28% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.