fix(rpc): describe Platform addresses consistently in validateaddress and getaddressinfo - #7578
Conversation
|
✅ Final review complete — no blockers (commit e8d0420) |
| "\nReturn information about the given Dash address.\n", | ||
| "\nReturn information about the given Dash address.\n" | ||
| "A DIP-18 Dash Platform address is described against the credit output script an asset\n" | ||
| "lock would carry for it, the same way getaddressinfo describes it.\n", |
There was a problem hiding this comment.
should remove disclaimer about getaddressinfo ; it seems irrelevant
There was a problem hiding this comment.
Done — dropped the getaddressinfo reference from the help text; it now just states the credit-output-script semantics on its own.
🤖 Posted autonomously by Claude on behalf of pasta.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughPlatform address handling now generates and returns the corresponding credit-output script from Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant validateaddress
participant PlatformAddress
participant RPCResponse
Client->>validateaddress: Submit Platform address
validateaddress->>PlatformAddress: Build credit-output script
PlatformAddress->>RPCResponse: Return scriptPubKey and isscript
PlatformAddress->>RPCResponse: Add L1 address details when available
validateaddress-->>Client: Return address information
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/rpc/output_script.cpp (1)
30-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the help text self-contained.
Remove the
getaddressinfocross-reference. Thevalidateaddresshelp should describe the credit-output behavior directly. Cross-RPC wording can become stale when the other RPC help changes.Proposed wording
- "lock would carry for it, the same way getaddressinfo describes it.\n", + "lock would carry for it.\n",🤖 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/rpc/output_script.cpp` around lines 30 - 32, Update the validateaddress help text in the RPC description to remove the getaddressinfo cross-reference, while retaining a self-contained direct description of the DIP-18 credit-output behavior for the supplied Dash address.
🤖 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/rpc/output_script.cpp`:
- Around line 30-32: Update the validateaddress help text in the RPC description
to remove the getaddressinfo cross-reference, while retaining a self-contained
direct description of the DIP-18 credit-output behavior for the supplied Dash
address.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 802e9070-c507-4deb-bc82-f6edd312cbaf
📒 Files selected for processing (3)
src/rpc/output_script.cppsrc/wallet/rpc/addresses.cpptest/functional/rpc_invalid_address_message.py
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The exact-head diff consistently reports the derived DIP-18 credit output script and destination type from validateaddress, clarifies the corresponding wallet RPC semantics, and adds functional coverage for both P2PKH and P2SH Platform addresses. No in-scope correctness, documentation, or test-coverage defects were identified.
Source: reviewer backends gpt-5.6-sol (general and dash-core-commit-history); final verifier backend gpt-5.6-sol. Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
… and getaddressinfo validateaddress deliberately omitted scriptPubKey for DIP-18 Platform addresses while getaddressinfo reported the derived credit output script along with ismine and isscript, so the two RPCs told inconsistent stories about the same address. Align them: validateaddress now also reports the credit output script an asset lock would carry for the address (plus isscript), with help text spelling out the semantics, and getaddressinfo's help clarifies that ismine/solvable refer to that script rather than to Platform identity ownership.
ab38b87 to
e8d0420
Compare
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
At exact head e8d0420, the implementation consistently derives the DIP-18 credit output script in validateaddress, reports the corresponding destination type, clarifies wallet-field semantics, and tests exact P2PKH and P2SH results. No in-scope correctness, documentation, or test-coverage defects were identified.
Source: reviewer backends gpt-5.6-sol (general) and gpt-5.6-sol (dash-core-commit-history); final verifier backend gpt-5.6-sol. Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
4 participants
Issue being fixed or feature implemented
Follow-up to #7294. Built on top of that PR's branch; only the last commit is new here — please review just
fix(rpc): describe Platform addresses consistently in validateaddress and getaddressinfo.With #7294, the two address-introspection RPCs tell inconsistent stories about the same DIP-18 Platform address:
validateaddressdeliberately omitsscriptPubKey("a Platform address has no layer-1 output script"), whilegetaddressinfodescribes the address against the credit output script an asset lock would carry for it, includingscriptPubKey,isscriptandismine— andismine: truethere is easy to misread as Platform identity ownership.What was done?
validateaddressnow also reports the credit output script (scriptPubKey) andisscriptfor a valid DIP-18 Platform address, matchinggetaddressinfo, with help text spelling out the semantics.getaddressinfohelp now clarifies thatismine/solvablerefer to the derived credit output script, not to ownership of a Platform identity.How Has This Been Tested?
Updated
rpc_invalid_address_message.py:check_platformnow asserts the exactscriptPubKeyandisscriptfor the DIP-18 P2PKH and P2SH test vectors (the P2SH script hash constant is decoded from the DIP-0018 test vector), alongside the existing normalization and error-field checks.Breaking Changes
validateaddressoutput for DIP-18 Platform addresses gainsscriptPubKeyandisscriptfields (previously absent). No behavior change for regular addresses.Checklist: