rpc: reject null for optional parameters #35582

pull RuslanProgrammer wants to merge 1 commits into bitcoin:master from RuslanProgrammer:rpc-reject-null-optional-params changing 5 files +18 −7

RuslanProgrammer commented at 1:30 PM on June 22, 2026: contributor

Treat explicitly passed null as missing for optional RPC parameters that are required in certain contexts.
DrahtBot added the label RPC/REST/ZMQ on Jun 22, 2026
DrahtBot commented at 1:30 PM on June 22, 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/35582.



Reviews

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

Type Reviewers

ACK achow101, maflcko, sedited

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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #35837 (rpc: fail scanblocks when block filter range is unavailable by MicSm)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
pinheadmz commented at 1:34 PM on June 22, 2026: member

Thanks for your submission! The description and code changes match a pattern we sometimes see with PRs that lean heavily on LLM coding assistants. Because the project has such limited human resources for code review, we require code generated by LLMs to be reviewed by the author who submits them first.

It would help to know why you decided to open this PR (did you encounter a bug? find an open issue in repo?), how you reviewed the generated code, what you personally understand about it, how you tested the code before and after the change, etc.

If you don't understand the changes you submitted, we should close this PR. There are many other more helpful ways to contribute to the project, and we look forward to working with you there!

Please also reconsider the value of the PR given how limited our resources are for code review.
RuslanProgrammer commented at 1:42 PM on June 22, 2026: contributor

Most rpc methods contain such checks, but some functions lack them. So this PR adds the missing checks.

You can check that this PR also adds tests to validate the changes.

Type	Reviewers
ACK	achow101, maflcko, sedited

in src/rpc/blockchain.cpp:2416 in abcc170979

2412 | @@ -2413,7 +2413,7 @@ static RPCMethod scantxoutset()
2413 |              throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan already in progress, use action \"abort\" or \"status\"");
2414 |          }
2415 |  
2416 | -        if (request.params.size() < 2) {
2417 | +        if (request.params.size() < 2 || request.params[1].isNull()) {

maflcko commented at 5:52 PM on June 30, 2026:

not sure about using positional args here, when named args can be used:

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index d78b82bce3..ad84c79682 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -2415,5 +2415,6 @@ static RPCMethod scantxoutset()
 
-        if (request.params.size() < 2) {
+        const UniValue& scan_objects{[&]() {
+            if (auto a{self.MaybeArg<UniValue>("scanobjects")}) return *a;
             throw JSONRPCError(RPC_MISC_ERROR, "scanobjects argument is required for the start action");
-        }
+        }()};
 
@@ -2424,3 +2425,3 @@ static RPCMethod scantxoutset()
         // loop through the scan objects
-        for (const UniValue& scanobject : request.params[1].get_array().getValues()) {
+        for (const UniValue& scanobject : scan_objects.get_array().getValues()) {
             FlatSigningProvider provider;

RuslanProgrammer commented at 8:29 AM on July 1, 2026:

Fixed

RuslanProgrammer force-pushed on Jul 1, 2026
rpc: reject null for optional parameters aeca061086
in src/rpc/output_script.cpp:324 in 283facdbfa
```
 321 | +            if (!desc->IsRange() && request.params.size() > 1 && !request.params[1].isNull()) {
 322 |                  throw JSONRPCError(RPC_INVALID_PARAMETER, "Range should not be specified for an un-ranged descriptor");
 323 |              }
 324 |  
 325 | -            if (desc->IsRange() && request.params.size() == 1) {
 326 | +            if (desc->IsRange() && (request.params.size() == 1 || request.params[1].isNull())) {
```
maflcko commented at 8:34 AM on July 1, 2026:

Same here, I don't think it makes sense to use confusing indexes when named args can be used?

RuslanProgrammer commented at 10:20 AM on July 1, 2026:

Fixed

RuslanProgrammer commented at 10:20 AM on July 1, 2026:

@maflcko, so maybe we should rewrite all indexes into named args?

maflcko commented at 10:03 AM on August 5, 2026:

@maflcko, so maybe we should rewrite all indexes into named args?

Yeah, that could make sense. Code like request.params.size() > 1 meaning bool{range} and request.params.size() == 1 meaning bool{!range} is wrong, ugly and confusing.

I am not sure how easy it will be to review. Maybe start with a single file to convert at first and then see if reviewers like it?
RuslanProgrammer force-pushed on Jul 1, 2026
achow101 commented at 7:50 PM on July 23, 2026: member

ACK aeca0610865ede44004b42a16ef6318245fe0644
sedited requested review from maflcko on Aug 5, 2026

maflcko commented at 10:00 AM on August 5, 2026: member

review ACK aeca0610865ede44004b42a16ef6318245fe0644 🥚

<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 aeca0610865ede44004b42a16ef6318245fe0644 🥚
WgKApVpXL8SyrPvZFh2Kliqiy6intF/LCxbdC9URkrqkglJxERR6NlW7UokmfAPOkHPuRGQxr+Nh98qqKRKXBQ==

</details>

maflcko added this to the milestone 32.0 on Aug 5, 2026
sedited approved
sedited commented at 10:23 AM on August 5, 2026: contributor

ACK aeca0610865ede44004b42a16ef6318245fe0644
sedited merged this on Aug 5, 2026
sedited closed this on Aug 5, 2026

Contributors

Review Requested
maflcko

Labels

RPC/REST/ZMQ

Milestone
32.0

rpc: reject null for optional parameters #35582

Code Coverage & Benchmarks

Reviews

Conflicts