Docs: align API documentation with current RPC surface#1729
Docs: align API documentation with current RPC surface#1729
Conversation
Co-authored-by: reuben <reuben@firo.org>
|
Cursor Agent can help with this pull request. Just |
WalkthroughThis PR adds comprehensive API documentation covering Firo's RPC commands, privacy protocols (Lelantus and Spark), masternode features, and configuration options organized across multiple functional categories. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
reubenyap
left a comment
reubenyap
left a comment
There was a problem hiding this comment.
PR #1729 Review: Add comprehensive API and component documentation
[1 file changed, +1732 / -0] — Adds a new doc/api-documentation.md covering RPC APIs, privacy protocols (Lelantus/Spark), LLMQ/ChainLocks, masternodes, core components, and configuration options.
Static Analysis
N/A — documentation-only PR (Markdown file, no C++ code).
Cross-Reference Verification
Verified RPC signatures, parameters, port numbers, address formats, and component APIs against the actual codebase. Most documentation is accurate, with the exceptions noted below.
Findings Summary
| # | Severity | File | Finding |
|---|---|---|---|
| 1 | HIGH | doc/api-documentation.md:1366 |
Spark mainnet prefix is sm, not sp |
| 2 | MEDIUM | doc/api-documentation.md:98 |
getblock verbosity is numeric (0/1/2), not boolean |
| 3 | MEDIUM | doc/api-documentation.md:992 |
7 RPCs missing from documentation |
| 4 | MEDIUM | doc/api-documentation.md:1371 |
Missing address format specs / regex patterns for transparent and Spark addresses |
Each finding is posted as an inline comment on the relevant code — click to view, resolve when addressed. Each comment includes a Prompt for AI agents that can be copy-pasted into Cursor/Copilot/Claude Code to auto-fix the issue.
Verified as Correct
- Transparent address prefix (
afor mainnet) ✓ - Spark testnet prefix (
st) ✓ - P2P port 8168, RPC port 8888 ✓
- Spark Names RPCs (
getsparknamedata,getsparknames) signatures and return formats ✓ - Privacy protocol component APIs (Lelantus JoinSplit, Spark SpendTransaction, keys) ✓
- Transaction type enum values ✓
- Masternode system components ✓
Review assisted by AI + codebase cross-referencing. Posted by @reubenyap.
| #### Spark Address Format | ||
|
|
||
| Spark addresses use Bech32 encoding: | ||
| - **Mainnet prefix:** `sp` |
There was a problem hiding this comment.
[HIGH] Spark mainnet address prefix is sm, not sp
The documentation states the mainnet prefix is sp, but the actual code in src/libspark/keys.cpp:196-198 constructs the HRP as:
hrp.push_back(ADDRESS_ENCODING_PREFIX); // 's' (from util.h:69)
hrp.push_back(network); // 'm' for mainnet (from util.h:73)So mainnet Spark addresses start with sm1..., not sp1.... The testnet prefix st is correct.
Suggested fix:
Change line 1366 to - **Mainnet prefix:** \sm`and update the example on line 1369 fromsp1qw508d...tosm1q...`.
🤖 Prompt for AI agents
"In file doc/api-documentation.md, the Spark Address Format section (around line 1366) incorrectly states the mainnet prefix is sp. Change it to sm. Also update the example address on line 1369 from sp1qw508d6qejxtdg4y5r3zarvary0c5xw7k... to an sm1... prefix. Reference: src/libspark/util.h defines ADDRESS_NETWORK_MAINNET = 'm' and ADDRESS_ENCODING_PREFIX = 's', producing HRP 'sm'."
|
|
||
| --- | ||
|
|
||
| #### `getblock "blockhash" [verbose]` |
There was a problem hiding this comment.
[MEDIUM] getblock verbosity parameter is numeric (0/1/2), not boolean
The documentation shows [verbose] as a boolean with default=true. The actual implementation in src/rpc/blockchain.cpp:943-1034 uses numeric verbosity levels:
0: hex-encoded data1: JSON object (default)2: JSON object with full transaction data (added by PR Getblock verbosity spark #1778)
Boolean is accepted for backward compatibility but the parameter should be documented as numeric.
Suggested fix:
Change the signature to getblock "blockhash" [verbosity] and document the three numeric levels (0, 1, 2) with default=1. Note that verbosity=2 includes decoded transaction data including Spark-specific fields.
🤖 Prompt for AI agents
"In file doc/api-documentation.md, the getblock RPC (around line 98) documents the second parameter as [verbose] (boolean, default=true). Change it to [verbosity] (numeric, optional, default=1) with three levels: 0 = hex data, 1 = JSON object, 2 = JSON with full transaction data. Update the result section to show the verbosity=2 output format. Boolean is still accepted for backwards compatibility but numeric is the primary interface."
|
|
||
| --- | ||
|
|
||
| ### Mobile RPCs |
There was a problem hiding this comment.
[MEDIUM] Several RPCs are missing from the documentation
The following RPCs exist in the codebase but are not documented:
Wallet RPCs:
signmessage "address" "message"— Sign a message with a transparent address (src/wallet/rpcwallet.cpp:1320)signmessagewithsparkaddress "sparkaddress" "message"— Sign a message with a Spark address (src/wallet/rpcwallet.cpp:1379, added in PR Implementation of signing/verification of message with Spark address #1784)
Utility RPCs:
verifymessage "address" "signature" "message"— Verify a signed message (src/rpc/misc.cpp:436)verifymessagewithsparkaddress "sparkaddress" "signature" "message"— Verify a Spark-signed message (src/rpc/misc.cpp:490, added in PR Implementation of signing/verification of message with Spark address #1784)signmessagewithprivkey "privkey" "message"— Sign with a private key directly (src/rpc/misc.cpp:553)
Address Index RPCs:
getspentinfo— Returns the txid and index where an output is spent (src/rpc/misc.cpp:1947, requires-spentindex)
Mobile RPCs:
getusedcoinstagstxhashes— Returns transaction hashes for used coin tags (src/rpc/misc.cpp:1629)
Suggested fix:
Add documentation sections for each missing RPC with their parameters, result formats, and examples.
🤖 Prompt for AI agents
"In file doc/api-documentation.md, add documentation for the following missing RPCs: (1) signmessage and verifymessage under Wallet RPCs and Utility sections respectively; (2) signmessagewithsparkaddress and verifymessagewithsparkaddress under a new 'Message Signing RPCs' section; (3) signmessagewithprivkey under Utility RPCs; (4) getspentinfo under Address Index RPCs; (5) getusedcoinstagstxhashes under Mobile RPCs. For each, include Arguments, Result format, and an Example. Reference the actual implementations in src/wallet/rpcwallet.cpp and src/rpc/misc.cpp for accurate parameter names and types."
|
|
||
| Example: `sp1qw508d6qejxtdg4y5r3zarvary0c5xw7k...` | ||
|
|
||
| --- |
There was a problem hiding this comment.
[MEDIUM] Missing address format specifications / regex patterns
The documentation mentions Spark addresses use Bech32 encoding but doesn't provide the actual validation rules or regex patterns that external developers would need to validate addresses. Similarly, there is no specification for transparent (base58check) Firo addresses.
For developer integration, the doc should include:
Transparent addresses (base58check):
- Mainnet P2PKH: version byte
0x52(82), addresses start witha, regex:^a[1-9A-HJ-NP-Za-km-z]{33}$ - Mainnet P2SH: version byte
0x07(7), addresses start with3or4 - Testnet P2PKH: version byte
0x41(65), addresses start withT
Spark addresses (bech32m):
- Mainnet HRP:
sm, regex:^sm1[a-z0-9]{100,}$ - Testnet HRP:
st, regex:^st1[a-z0-9]{100,}$ - Decoded payload size: exactly
2 * GroupElement::serialize_size + AES_BLOCKSIZEbytes - Validated via
bech32::decode()+ custom size/HRP checks insrc/libspark/keys.cpp:207-249
Suggested fix:
Add an "Address Formats" section documenting both transparent and Spark address formats with version bytes, prefixes, and validation regex patterns.
🤖 Prompt for AI agents
"In file doc/api-documentation.md, after the Spark Address Format section (around line 1371), add a comprehensive 'Address Validation' subsection. Include: (1) Transparent address format: base58check with mainnet P2PKH version byte 0x52 (prefix 'a'), P2SH version byte 0x07, testnet P2PKH version byte 0x41 (prefix 'T'). Include regex patterns. (2) Spark address format: bech32m encoding, mainnet HRP 'sm', testnet HRP 'st', regtest HRP 'sr'. Include regex patterns and note the decoded payload size requirement. Reference src/chainparams.cpp for base58 prefixes and src/libspark/keys.cpp + src/libspark/util.h for Spark encoding."
🤖 Combined AI Fix PromptVerify each finding against the current code and only fix it if needed.
|
reubenyap
added
documentation
Co-authored-by: Reuben Yap <reuben@firo.org>
cursor
Bot
changed the title
Co-authored-by: Reuben Yap <reuben@firo.org>
cursor
Bot
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
doc/api-documentation.md (2)
576-634:⚠️ Potential issue | 🟠 MajorA few RPCs requested in prior review are still missing from detailed docs/index.
signmessagewithsparkaddress,verifymessagewithsparkaddress, andgetspentinfoare still not documented here, andgetspentinfois also absent from the command index section. This leaves the “current RPC surface” documentation incomplete.Suggested additions
+#### `signmessagewithsparkaddress "sparkaddress" "message"` +Sign a message with a Spark address. + +#### `verifymessagewithsparkaddress "sparkaddress" "signature" "message"` +Verify a message signed with a Spark address. ... +#### `getspentinfo {"txid":"...","index":N}` +Returns spending transaction reference for an outpoint. + +**Notes:** +- Requires `-spentindex`#### `util` -`createmultisig`, ..., `signmessagewithprivkey`, `validateaddress`, `verifymessage` +`createmultisig`, ..., `signmessagewithprivkey`, `validateaddress`, `verifymessage`, `verifymessagewithsparkaddress` #### `wallet` -..., `signmessage`, ... +..., `signmessage`, `signmessagewithsparkaddress`, ... #### `addressindex` -`getAddressNumWBalance`, ..., `getzerocoinpoolbalance` +`getAddressNumWBalance`, ..., `getspentinfo`, ..., `getzerocoinpoolbalance`Also applies to: 1214-1329
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/api-documentation.md` around lines 576 - 634, Add documentation entries for the missing RPCs: signmessagewithsparkaddress, verifymessagewithsparkaddress, and getspentinfo; include their argument lists, result descriptions, notes (e.g., wallet/unlocked requirements or address/key constraints), and example CLI calls mirroring the style used for signmessage/signmessagewithprivkey/verifymessage, and also add getspentinfo to the command index section so the “current RPC surface” is complete.
103-140:⚠️ Potential issue | 🟠 Major
getblockstill documents deprecated boolean interface as primary.This section still uses
[verbose](boolean) instead of[verbosity](numeric, default1) and does not document verbosity2output shape. That keeps the RPC contract documentation out of sync with current behavior.Suggested doc patch
-#### `getblock "blockhash" [verbose]` +#### `getblock "blockhash" [verbosity]` Returns block data for the given block hash. **Arguments:** 1. `blockhash` (string, required) - The block hash -2. `verbose` (boolean, optional, default=true) - `true` for a JSON object, `false` for hex-encoded block data +2. `verbosity` (numeric, optional, default=1) + - `0`: hex-encoded block data + - `1`: JSON object with transaction IDs + - `2`: JSON object with fully decoded transaction data (including Spark fields) + - Boolean is accepted for backward compatibility (`false`=`0`, `true`=`1`) -**Result (verbose=false):** `"data"` (string) - The serialized block encoded as hex +**Result (verbosity=0):** `"data"` (string) - The serialized block encoded as hex -**Result (verbose=true):** +**Result (verbosity=1):** ... + +**Result (verbosity=2):** +```json +{ + "...": "...", + "tx": [ + { + "txid": "...", + "vin": [...], + "vout": [...], + "type": "...", + "spark": {...} + } + ] +} +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/api-documentation.md` around lines 103 - 140, Update the getblock RPC docs to replace the deprecated boolean parameter `[verbose]` with the numeric `[verbosity]` (default=1) and document both verbosity=1 and verbosity=2 outputs; change the Arguments section to show `verbosity` (numeric, optional, default=1), keep the current JSON example as the verbosity=1 shape (where "tx" is an array of txids) and add a new JSON example for verbosity=2 showing full tx objects including keys like "txid", "vin", "vout", "type", and "spark"; also update any inline text that mentions `verbose`/true/false to refer to `verbosity`/1/2 and ensure the Result descriptions clearly distinguish verbosity=false/1 (hex or txid list) vs verbosity=2 (expanded tx objects).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/api-documentation.md`:
- Around line 15-17: TOC entry "Privacy RPCs" points to the non-existent
fragment "#privacy-rpcs"; either add a heading whose text will generate that
anchor (e.g., a section titled "Privacy RPCs") or change the TOC link to the
exact slug of the existing heading; update the TOC line containing "Privacy
RPCs" (the fragment "#privacy-rpcs") to match the actual heading slug or add the
heading with the exact text so the fragment resolves.
---
Duplicate comments:
In `@doc/api-documentation.md`:
- Around line 576-634: Add documentation entries for the missing RPCs:
signmessagewithsparkaddress, verifymessagewithsparkaddress, and getspentinfo;
include their argument lists, result descriptions, notes (e.g., wallet/unlocked
requirements or address/key constraints), and example CLI calls mirroring the
style used for signmessage/signmessagewithprivkey/verifymessage, and also add
getspentinfo to the command index section so the “current RPC surface” is
complete.
- Around line 103-140: Update the getblock RPC docs to replace the deprecated
boolean parameter `[verbose]` with the numeric `[verbosity]` (default=1) and
document both verbosity=1 and verbosity=2 outputs; change the Arguments section
to show `verbosity` (numeric, optional, default=1), keep the current JSON
example as the verbosity=1 shape (where "tx" is an array of txids) and add a new
JSON example for verbosity=2 showing full tx objects including keys like "txid",
"vin", "vout", "type", and "spark"; also update any inline text that mentions
`verbose`/true/false to refer to `verbosity`/1/2 and ensure the Result
descriptions clearly distinguish verbosity=false/1 (hex or txid list) vs
verbosity=2 (expanded tx objects).
🪄 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: ed188c95-940d-4714-a49a-570828a042ff
📒 Files selected for processing (1)
doc/api-documentation.md
| - [Privacy RPCs](#privacy-rpcs) | ||
| - [Mobile RPCs](#mobile-rpcs) | ||
| - [Address Index RPCs](#address-index-rpcs) |
There was a problem hiding this comment.
TOC link fragment is invalid for Privacy RPCs.
The TOC links to #privacy-rpcs, but no heading generates that anchor, so the link is broken.
Suggested fix
- - [Privacy RPCs](`#privacy-rpcs`)
+ - [Privacy RPCs - Lelantus](`#privacy-rpcs---lelantus`)
+ - [Privacy RPCs - Spark](`#privacy-rpcs---spark`)🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 15-15: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc/api-documentation.md` around lines 15 - 17, TOC entry "Privacy RPCs"
points to the non-existent fragment "#privacy-rpcs"; either add a heading whose
text will generate that anchor (e.g., a section titled "Privacy RPCs") or change
the TOC link to the exact slug of the existing heading; update the TOC line
containing "Privacy RPCs" (the fragment "#privacy-rpcs") to match the actual
heading slug or add the heading with the exact text so the fragment resolves.
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
2 participants
PR intention
Generate comprehensive documentation for Firo's public APIs, functions, and components, including usage examples and instructions. This PR introduces a new
api-documentation.mdfile.Code changes brief
A new markdown file (
doc/api-documentation.md) has been added, containing detailed documentation for RPCs (Blockchain, Wallet, Network, Mining, Masternode, Privacy, Mobile, Address Index), privacy protocols (Lelantus, Spark), LLMQ/ChainLocks, Masternodes, and core components, along with practical examples, error codes, and configuration options. No existing code files were modified.Note
Low Risk
Documentation-only change that adds a new markdown file; no runtime behavior or security-sensitive code paths are modified.
Overview
Adds a new
doc/api-documentation.mdproviding a consolidated reference for the current RPC surface (including a generated complete command index), selected detailed RPC call semantics, and usage examples.Also documents key protocol/component concepts (Lelantus/Spark, ChainLocks/LLMQ, masternodes, core wallet/transaction/script types) plus common error codes and configuration flags; no source code changes are included.
Written by Cursor Bugbot for commit 3d6a5ee. This will update automatically on new commits. Configure here.