rpc, wallet: fix invalid JSON in HelpExampleRpc curl examples #35868

GuTS805 commented at 1:16 AM on August 3, 2026: contributor

Several HelpExampleRpc call sites reused CLI-style argument strings verbatim instead of valid JSON — missing commas, bare unquoted words, or single backslashes that are not valid JSON escapes. As a result the documented curl command for 14 RPCs (getblockfrompeer, addnode, addconnection, sendmsgtopeer, restorewallet, getmempoolcluster, importmempool, getindexinfo, listlabels, unloadwallet, createwalletdescriptor, addhdkey, loadwallet, listunspent) fails to parse as JSON if copy-pasted as-is. Also fixes a stray trailing quote in the restorewallet named-argument examples.

This was previously raised in #31275, which sipa confirmed at runtime by adding a UniValue::read check, but that PR was closed unmerged. Since then two more examples broke the same way (getmempoolcluster, addhdkey), which is why this adds a permanent regression check to rpc_help.py::dump_help() instead of just fixing the current list.

Fixes #35864.

DrahtBot commented at 1:16 AM on August 3, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35868.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	maflcko, sedited

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

in src/wallet/rpc/backup.cpp:628 in 88d9e18db7

 624 | @@ -625,9 +625,9 @@ RPCMethod restorewallet()
 625 |          },
 626 |          RPCExamples{
 627 |              HelpExampleCli("restorewallet", "\"testwallet\" \"home\\backups\\backup-file.bak\"")
 628 | -            + HelpExampleRpc("restorewallet", "\"testwallet\" \"home\\backups\\backup-file.bak\"")
 629 | -            + HelpExampleCliNamed("restorewallet", {{"wallet_name", "testwallet"}, {"backup_file", "home\\backups\\backup-file.bak\""}, {"load_on_startup", true}})
 630 | -            + HelpExampleRpcNamed("restorewallet", {{"wallet_name", "testwallet"}, {"backup_file", "home\\backups\\backup-file.bak\""}, {"load_on_startup", true}})
 631 | +            + HelpExampleRpc("restorewallet", "\"testwallet\", \"home\\\\backups\\\\backup-file.bak\"")

maflcko commented at 6:51 AM on August 3, 2026:

I don't think the \\b was caught (or will be caught in the future) as an issue by the json-parse test?

In any place where manual escaping is needed, it seems easier to just use raw strings directly:

R"("testwallet", "home\\backups\\backup-file.bak")";

GuTS805 force-pushed on Aug 3, 2026

GuTS805 commented at 7:30 AM on August 3, 2026: contributor

Good catch, thanks. You're right that \b would silently pass the json.loads() check since it's a technically-valid (but wrong) JSON escape. Switched both this one and the loadwallet example to raw string literals as suggested, pushed in the latest commit.

in src/wallet/rpc/wallet.cpp:864 in 67d7b0e16d

 860 | @@ -861,7 +861,7 @@ RPCMethod addhdkey()
 861 |              },
 862 |          },
 863 |          RPCExamples{
 864 | -            HelpExampleCli("addhdkey", "xprv") + HelpExampleRpc("addhdkey", "xprv")
 865 | +            HelpExampleCli("addhdkey", "xprv") + HelpExampleRpc("addhdkey", "\"xprv\"")

maflcko commented at 8:01 AM on August 3, 2026:

I'd say generally it seems best to not spend time on manually escaping all quotes and backslashes in all those strings after a test failure.

It seems better to use r-strings like R"("xprv")" consistently (at least for all lines required to be touched in this pull anyway)?

GuTS805 force-pushed on Aug 3, 2026

GuTS805 commented at 10:33 AM on August 3, 2026: contributor

Pushed an update addressing the review feedback and the CI failures from the previous run:

Converted all lines touched in this PR to raw string literals for consistency (per the suggestion above), avoiding manual quote/backslash escaping.
While investigating the CI failure, found one more pre-existing bug of the same class that my earlier scan had missed: listunspent's example (src/wallet/rpc/coins.cpp:517) was missing a comma between 9999999 and the addresses array, since it's built via string concatenation with EXAMPLE_ADDRESS rather than a plain literal. Fixed in this update.
Reordered the two commits (fix first, test second), since the ci-test-each-commit job runs the full test suite standalone on every commit, and the previous test-then-fix ordering was correctly failing on the intermediate ancestor commit.

All 154 HelpExampleRpc call sites in the codebase now produce valid JSON (verified locally); the one remaining call site not covered is an intentional fuzz-harness invocation with random input (src/test/fuzz/string.cpp), which is out of scope.

DrahtBot added the label CI failed on Aug 3, 2026

maflcko commented at 12:09 PM on August 3, 2026: member

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

GuTS805 force-pushed on Aug 3, 2026

GuTS805 commented at 3:32 PM on August 3, 2026: contributor

Done, squashed into a single commit.

GuTS805 commented at 11:03 AM on August 4, 2026: contributor

The 2 remaining failures (NetBSD Cross, riscv32 bare metal) look unrelated to this change. NetBSD Cross fails on a 404 from cdn.netbsd.org fetching the SDK tarball, and riscv32 fails during the GCC mirror submodule clone with no clear error, before any of our code is even built/tested. Happy to have these re-run if a maintainer can trigger that.

maflcko closed this on Aug 4, 2026

maflcko reopened this on Aug 4, 2026

DrahtBot removed the label CI failed on Aug 4, 2026

DrahtBot added the label CI failed on Aug 6, 2026

DrahtBot commented at 12:05 AM on August 6, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task riscv32 bare metal, static libbitcoin_consensus: https://github.com/bitcoin/bitcoin/actions/runs/30805638058/job/92449829226</sub> <sub>LLM reason (✨ experimental): CI failed because submodule cloning of https://sourceware.org/git/binutils-gdb.git was rate-limited (HTTP 429) and exited with code 2.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on Aug 7, 2026

sedited commented at 10:21 AM on August 27, 2026: contributor

Did you manually check whether these example commands now work?

GuTS805 commented at 11:01 AM on August 27, 2026: contributor

@sedited Not end-to-end against a live node. That would need matching wallet. which is a different check than the bug being fixed here. What I did verify: a script that renders the exact JSON payload for every HelpExampleRpc call (substituting EXAMPLE_ADDRESS/EXAMPLE_DESCRIPTOR where used) and confirms it parses correctly, plus the CI regression test added in this PR, which validates the same thing against real compiled-node output, currently green. None of the fixes changed argument order/count/type, only the JSON syntax, so each RPC-form example now matches its already-working CLI-form sibling exactly. If you hit a specific example that actually fails, let me know which one.

sedited commented at 11:17 AM on August 27, 2026: contributor

That's fair, I guess I just needed to hear again that this only verifies the JSON syntax. Can you drop the last (outdated) sentence from the pull request description?

GuTS805 commented at 11:40 AM on August 27, 2026: contributor

@sedited Done 👍

sedited approved

sedited commented at 11:59 AM on August 27, 2026: contributor

ACK d831908a4030be9de115dc86ef8f4891bb0e2c3f

in src/wallet/rpc/coins.cpp:517 in d831908a40

 513 | @@ -514,7 +514,7 @@ RPCMethod listunspent()
 514 |                  RPCExamples{
 515 |                      HelpExampleCli("listunspent", "")
 516 |              + HelpExampleCli("listunspent", "6 9999999 \"[\\\"" + EXAMPLE_ADDRESS[0] + "\\\",\\\"" + EXAMPLE_ADDRESS[1] + "\\\"]\"")
 517 | -            + HelpExampleRpc("listunspent", "6, 9999999 \"[\\\"" + EXAMPLE_ADDRESS[0] + "\\\",\\\"" + EXAMPLE_ADDRESS[1] + "\\\"]\"")
 518 | +            + HelpExampleRpc("listunspent", "6, 9999999, \"[\\\"" + EXAMPLE_ADDRESS[0] + "\\\",\\\"" + EXAMPLE_ADDRESS[1] + "\\\"]\"")

maflcko commented at 12:58 PM on August 27, 2026:

this is valid json, but obviously still the wrong schema, and won't work when actually used

GuTS805 commented at 4:42 PM on August 27, 2026:

@maflcko Fixed. Can you review it again.

maflcko commented at 12:59 PM on August 27, 2026: member

Used --color-words --word-diff-regex=. -U0.

Seems fine to merge, but this is still wrong.

review ACK d831908a4030be9de115dc86ef8f4891bb0e2c3f 🌏

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK d831908a4030be9de115dc86ef8f4891bb0e2c3f 🌏
QmVhlkARPCz0T2hdCmvTKm2DOpkmOvUxUGgdp2JCbUhvIGSAX2YbSmSJTuM+aDxQF0F4H1gNsdjaXAEiPt2pCw==

</details>

maflcko commented at 4:55 PM on August 27, 2026: member

This should use an R-string, like the others to avoid escapes ( and use tfm::format)

maflcko commented at 4:55 PM on August 27, 2026: member

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

rpc, wallet, test: fix invalid JSON in HelpExampleRpc curl examples

Several HelpExampleRpc call sites reused CLI-style argument strings
verbatim (missing commas, bare unquoted words, or single backslashes
that are not valid JSON escapes), producing curl examples that fail
JSON parsing as documented. Also fixes a stray trailing quote in the
restorewallet named-argument examples, a missing comma in the
listunspent example, and a wrong-schema string-instead-of-array
listunspent argument caught in review.

Lines touched are converted to raw string literals (or strprintf with a
raw string template) throughout, for consistency and to avoid manual
quote/backslash escaping.

Adds a regression check to rpc_help.py::dump_help() so this class of
bug can't silently reappear.

21d4e0ba75

GuTS805 force-pushed on Aug 27, 2026

GuTS805 commented at 5:07 PM on August 27, 2026: contributor

@maflcko Done 👍. Switched to strprintf with a raw string template, and squashed back into one commit.

maflcko commented at 5:18 PM on August 27, 2026: member

review ACK 21d4e0ba759bb1024c5cc14c76b4f2963f252007 🚝

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 21d4e0ba759bb1024c5cc14c76b4f2963f252007 🚝
+TJalLrK7haW5KrKK/2XN70zCoxGwocGHr6z8s+HEyBthUS5MeP7GFmOvr4N45L3uRS52nLDtv3HvR0I/4CzAQ==

</details>

DrahtBot requested review from sedited on Aug 27, 2026

sedited approved

sedited commented at 10:40 AM on August 28, 2026: contributor

ACK 21d4e0ba759bb1024c5cc14c76b4f2963f252007

sedited merged this on Aug 29, 2026

sedited closed this on Aug 29, 2026

GuTS805 deleted the branch on Aug 29, 2026