docs: clarify ProTx platform address RPC help#7375
Draft
thepastaclaw wants to merge 1 commit into
Draft
Conversation
The RPC help for `protx register_evo` / `update_service_evo` (and the register_fund_evo / register_prepare_evo variants) documented `coreP2PAddrs`, `platformP2PAddrs`, and `platformHTTPSAddrs` only as `["ADDR:PORT"]` arrays, but a legacy MnNetInfo / version-2 ProTx accepts a bare port for the platform fields (the IP is inherited from the core address) and only a single entry for the core field. Users following the help would hit a terse `ProTx version only supports ports` error with no example of what was actually accepted. Update the help strings to call out the legacy vs. Extended addresses (ExtNetInfo) format, and reword the rejection in `ProcessNetInfoPlatform` to show an example port and point at Extended addresses for the array form. Update the matching functional-test expectations and pin the full new error string in one assertion to prevent regression of the hint. Fixes dashpay#7370
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
This pull request has conflicts, please rebase. |
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThe change improves clarity of error feedback for legacy (version-2 / MnNetInfo) ProTx address parameters. In Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@test/functional/rpc_netinfo.py`:
- Around line 275-278: The error message validation in the first register_mn
call on line 276 is missing the "Invalid param for platformP2PAddrs," prefix
that the validator actually returns. Additionally, the second register_mn call
on line 278 contains an outdated truncated error message text. Update both error
message strings in these two register_mn calls to match the complete and current
validator error messages, ensuring that the first one includes the "Invalid
param for platformP2PAddrs," prefix and the second one uses the full validator
message text instead of the truncated version.
- Around line 284-293: The error message expectations in all four register_mn
function calls need to be updated to match the new error message format. The
current error messages only contain the old prefix text, but they should be
updated to include the new example-port and ExtNetInfo wording that is now
emitted by ProcessNetInfoPlatform. Update each of the four error message strings
(passed as the last argument to each register_mn call) to include the additional
new wording that ProcessNetInfoPlatform now generates for invalid array-wrapped
port parameters in platformP2PAddrs and platformHTTPSAddrs.
- Around line 294-304: The test assertions for the empty-core and
partial-platform registration cases in the register_mn calls are using outdated
error message formats. Update all error message strings in the register_mn
method calls within this test section to use the full updated error message
format that includes the bare-port example (e.g., "Invalid param for
platformP2PAddrs, this ProTx version only accepts a bare port number"), ensuring
they match the current validator output being checked in the test.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8f06aaf0-59e9-4cbf-987c-c263d5c0fc9f
📒 Files selected for processing (3)
src/rpc/evo.cppsrc/rpc/evo_util.cpptest/functional/rpc_netinfo.py
Comment on lines
275
to
+278
| self.node_evo.register_mn(self, False, f"127.0.0.1:{self.node_evo.mn.nodePort}", f"127.0.0.1:{DEFAULT_PORT_PLATFORM_P2P}", DEFAULT_PORT_PLATFORM_HTTP, | ||
| -8, "Invalid param for platformP2PAddrs, ProTx version only supports ports") | ||
| -8, "this ProTx version only accepts a bare port number (e.g. 26656); the \"ADDR:PORT\" / address-array form requires an Extended addresses (ExtNetInfo) ProTx") | ||
| self.node_evo.register_mn(self, False, f"127.0.0.1:{self.node_evo.mn.nodePort}", [f"127.0.0.1:{DEFAULT_PORT_PLATFORM_P2P}"], DEFAULT_PORT_PLATFORM_HTTP, | ||
| -8, "Invalid param for platformP2PAddrs, ProTx version only supports ports") | ||
| -8, "Invalid param for platformP2PAddrs, this ProTx version only accepts a bare port number") |
There was a problem hiding this comment.
Refresh the first legacy P2P rejection string.
Line 276 is missing the Invalid param for platformP2PAddrs, prefix, and Line 278 still checks the old truncated text. Update both to the full validator message so this case actually covers the new wording.
🤖 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 `@test/functional/rpc_netinfo.py` around lines 275 - 278, The error message
validation in the first register_mn call on line 276 is missing the "Invalid
param for platformP2PAddrs," prefix that the validator actually returns.
Additionally, the second register_mn call on line 278 contains an outdated
truncated error message text. Update both error message strings in these two
register_mn calls to match the complete and current validator error messages,
ensuring that the first one includes the "Invalid param for platformP2PAddrs,"
prefix and the second one uses the full validator message text instead of the
truncated version.
Comment on lines
284
to
293
| # Port numbers may not be wrapped in arrays, either as integers or strings | ||
| self.node_evo.register_mn(self, False, f"127.0.0.1:{self.node_evo.mn.nodePort}", [DEFAULT_PORT_PLATFORM_P2P], DEFAULT_PORT_PLATFORM_HTTP, | ||
| -8, "Invalid param for platformP2PAddrs, ProTx version only supports ports") | ||
| -8, "Invalid param for platformP2PAddrs, this ProTx version only accepts a bare port number") | ||
| self.node_evo.register_mn(self, False, f"127.0.0.1:{self.node_evo.mn.nodePort}", [f"{DEFAULT_PORT_PLATFORM_P2P}"], DEFAULT_PORT_PLATFORM_HTTP, | ||
| -8, "Invalid param for platformP2PAddrs, ProTx version only supports ports") | ||
| -8, "Invalid param for platformP2PAddrs, this ProTx version only accepts a bare port number") | ||
| self.node_evo.register_mn(self, False, f"127.0.0.1:{self.node_evo.mn.nodePort}", DEFAULT_PORT_PLATFORM_P2P, [DEFAULT_PORT_PLATFORM_HTTP], | ||
| -8, "Invalid param for platformHTTPSAddrs, ProTx version only supports ports") | ||
| -8, "Invalid param for platformHTTPSAddrs, this ProTx version only accepts a bare port number") | ||
| self.node_evo.register_mn(self, False, f"127.0.0.1:{self.node_evo.mn.nodePort}", DEFAULT_PORT_PLATFORM_P2P, [f"{DEFAULT_PORT_PLATFORM_HTTP}"], | ||
| -8, "Invalid param for platformHTTPSAddrs, ProTx version only supports ports") | ||
| -8, "Invalid param for platformHTTPSAddrs, this ProTx version only accepts a bare port number") | ||
|
|
There was a problem hiding this comment.
Update the array-wrapped port expectations.
These four assertions still only match the old prefix. They should include the new example-port and ExtNetInfo wording emitted by ProcessNetInfoPlatform, otherwise these cases won’t verify the new behavior.
🤖 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 `@test/functional/rpc_netinfo.py` around lines 284 - 293, The error message
expectations in all four register_mn function calls need to be updated to match
the new error message format. The current error messages only contain the old
prefix text, but they should be updated to include the new example-port and
ExtNetInfo wording that is now emitted by ProcessNetInfoPlatform. Update each of
the four error message strings (passed as the last argument to each register_mn
call) to include the additional new wording that ProcessNetInfoPlatform now
generates for invalid array-wrapped port parameters in platformP2PAddrs and
platformHTTPSAddrs.
Comment on lines
294
to
+304
| # coreP2PAddrs cannot be empty when registering a masternode without specifying platform fields | ||
| self.node_evo.register_mn(self, False, "", "", "", | ||
| -8, "Invalid param for platformP2PAddrs, ProTx version only supports ports") | ||
| -8, "Invalid param for platformP2PAddrs, this ProTx version only accepts a bare port number") | ||
| self.node_evo.register_mn(self, False, "", "", DEFAULT_PORT_PLATFORM_HTTP, | ||
| -8, "Invalid param for platformP2PAddrs, ProTx version only supports ports") | ||
| -8, "Invalid param for platformP2PAddrs, this ProTx version only accepts a bare port number") | ||
| self.node_evo.register_mn(self, False, f"127.0.0.1:{self.node_evo.mn.nodePort}", "", "", | ||
| -8, "Invalid param for platformP2PAddrs, cannot be empty if other fields populated") | ||
| self.node_evo.register_mn(self, False, f"127.0.0.1:{self.node_evo.mn.nodePort}", "", DEFAULT_PORT_PLATFORM_HTTP, | ||
| -8, "Invalid param for platformP2PAddrs, cannot be empty if other fields populated") | ||
| self.node_evo.register_mn(self, False, "", DEFAULT_PORT_PLATFORM_P2P, "", | ||
| -8, "Invalid param for platformHTTPSAddrs, ProTx version only supports ports") | ||
| -8, "Invalid param for platformHTTPSAddrs, this ProTx version only accepts a bare port number") |
There was a problem hiding this comment.
Bring the empty-core/partial-platform cases up to date.
These assertions still expect the short legacy text. Use the full Invalid param for ... message, including the bare-port example, so the updated validator output is actually checked here.
🤖 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 `@test/functional/rpc_netinfo.py` around lines 294 - 304, The test assertions
for the empty-core and partial-platform registration cases in the register_mn
calls are using outdated error message formats. Update all error message strings
in the register_mn method calls within this test section to use the full updated
error message format that includes the bare-port example (e.g., "Invalid param
for platformP2PAddrs, this ProTx version only accepts a bare port number"),
ensuring they match the current validator output being checked in the test.
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
1 participant
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.
Issue being fixed or feature implemented
Fixes #7370.
protx register_evoandprotx update_service_evoaccepted two different platform address formats depending on the ProTx net-info version, but the RPC help only described the Extended-address array form. When a legacy MnNetInfo / version-2 ProTx rejected"ADDR:PORT"or array input forplatformP2PAddrs/platformHTTPSAddrs, the error only said the version supported ports, without showing the expected shape.What was done?
coreP2PAddrs,platformP2PAddrs,platformHTTPSAddrs, and update variants to explain legacy MnNetInfo/version-2 bare-port input versus Extended addresses (ExtNetInfo)"ADDR:PORT"array input.26656for Platform P2P and443for Platform HTTPS.rpc_netinfo.pyexpectations so the clearer messages are covered, including the HTTPS example-port case caught during review.How Has This Been Tested?
Tested on macOS arm64 in the
fix-protx-netinfo-helpworktree.make -j8 src/dashdtest/functional/rpc_netinfo.pycode-review dashpay/dash upstream/develop fix-protx-netinfo-help "Fix issue #7370 by clarifying protx register/update platform address help and legacy v2 error messages"→ shipBreaking Changes
None. This changes RPC help/error text and matching functional-test expectations only; parsing behavior is unchanged.
Checklist: