rpc: Populate some skip type check args for openrpc #35946

sedited commented at 7:07 PM on August 10, 2026: contributor

This was initially motivated by testing the dump of the schema against open-rpc-generator, which crashed with:

open-rpc-generator generate -t client -l rust -n bitcoin_client -d ./openrpc.gen.json -o ./generated
There was error at generator runtime:
TypeError: Cannot convert undefined or null to object

The changes here fix this crash (albeit perfectly valid existing schema), but I think creating a more complete output is helpful on its own. The openrpc schema dumps can eventually be re-used for the rpc docs and to track rpc interface changes more accurately. Adding the CreateTxDoc outputs section seems useful for that.

DrahtBot added the label RPC/REST/ZMQ on Aug 10, 2026

sedited requested review from willcl-ark on Aug 10, 2026

DrahtBot commented at 7:07 PM on August 10, 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/35946.

Reviews

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

Type	Reviewers
ACK	willcl-ark

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

in src/rpc/server.cpp:412 in 8381ab6de5

 406 | @@ -403,14 +407,19 @@ UniValue OpenRPCArgSchema(const RPCArg& arg, bool include_hidden)
 407 |          }
 408 |          schema.pushKV("type", "object");
 409 |          schema.pushKV("properties", std::move(properties));
 410 | -        schema.pushKV("additionalProperties", false);
 411 | +        if (!in_skip_type_check) {
 412 | +            schema.pushKV("additionalProperties", false);
 413 | +        }

willcl-ark commented at 12:40 PM on August 12, 2026:

In 019ee7d5e66b74eac42199f64e08cd0e90af4603bb3c105e294665ea4b411219R410

I am not so sure about this. I think we still want plain schema.pushKV("additionalProperties", false); here, so that a schema consumer would not think that something like [{"data": "00", "extra": 1}] was valid per the schema.

The RPC would reject this (in either case) as we only accept one key for each array item, but as we are tightening this I think it makes sense to do so here so we are even more aligned with what the RPC server accepts.

sedited commented at 1:16 PM on August 12, 2026:

I went back and forth on this a bit a few days ago, and I couldn't really make up my mind. It is a bit confusing: When passed as an array, we don't accept mixed keys, when passed as an object, we do. Keys that aren't valid addresses are rejected (but only later on in the logic). I think your suggestion is good. In the end we should just stick closest to what the code actually rejects.

willcl-ark commented at 12:42 PM on August 12, 2026: member

Concept ACK.

Looks pretty good to me, just left one comment about tightening the openRPC schema for array elements to more closely match our server.

rpc: Handle skip type args for openrpc

Instead of filling them in with empty object and array args be a bit
more friendly to the consumer by giving type hints, while retaining type
flexibility:

Keep the empty object, but fill the array with the hinted at types by
recursing through them. Add an additional argument to the openrpc
functions (`in_skip_type_check`) to keep track of when a loosely typed
argument is under evaluation. Note that the array is evaluated stricter
than before: If it contains items, they need to match the nested
objects.

The change from oneOf->anyOf and removing additionalProperties: false
should still convey to the reader that this is a loosely typed object
and prevent collisions between these loosely-typed schemas.

This was initially motivated by testing the dump of the schema against
open-rpc-generator, which crashed with:

```
open-rpc-generator generate -t client -l rust -n bitcoin_client -d ./openrpc.gen.json -o ./generated
There was error at generator runtime:
TypeError: Cannot convert undefined or null to object
```

The changes here fix this crash, but I think creating a friendlier
output is helpful on its own.

This patch changes the schema exported from CreateTxDoc as follows:

```diff

diff -U6 <(jq '.methods[] | select(.name == "createrawtransaction")' dump.json) \
     <(jq '.methods[] | select(.name == "createrawtransaction")' new_dump.json)
@@ -37,13 +37,43 @@
     {
       "name": "outputs",
       "required": true,
       "schema": {
         "oneOf": [
           {
-            "type": "array"
+            "type": "array",
+            "items": {
+              "anyOf": [
+                {
+                  "type": "object",
+                  "additionalProperties": {
+                    "oneOf": [
+                      {
+                        "type": "number"
+                      },
+                      {
+                        "type": "string"
+                      }
+                    ]
+                  }
+                },
+                {
+                  "type": "object",
+                  "properties": {
+                    "data": {
+                      "type": "string",
+                      "pattern": "^[0-9a-fA-F]+$",
+                      "description": "A key-value pair. The key must be \"data\", the value is hex-encoded data that becomes a part of an OP_RETURN output"
+                    }
+                  },
+                  "required": [
+                    "data"
+                  ]
+                }
+              ]
+            }
           },
           {
             "type": "object"
           }
         ]
       },
```

c020c21d54

rpc: Surface OBJ_USER_KEYS description for openrpc

While this is usually used where the types are not enforced strictly,
adding the description is both useful to the developer implementing a
client and for potentially using the openrpc output as a basis for
documentation.

```diff
diff interim_dump.json new_dump.json
465c465,466
<                           }
---
>                           },
>                           "description": "A key-value pair. The key (string) is the bitcoin address,\nthe value (float or string) is the amount in BTC"
777c778,779
<                       }
---
>                       },
>                       "description": "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in BTC"
903c905,906
<                       }
---
>                       },
>                       "description": "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in BTC"
12915c12918,12919
<                           }
---
>                           },
>                           "description": "A key-value pair. The key (string) is the bitcoin address,\nthe value (float or string) is the amount in BTC"
13692c13696,13697
<                       }
---
>                       },
>                       "description": "A key-value pair. The key (string) is the bitcoin address,\nthe value (float or string) is the amount in BTC"
14002c14007,14008
<                   }
---
>                   },
>                   "description": "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in BTC"
14254c14260,14261
<             }
---
>             },
>             "description": "The bitcoin address is the key, the numeric amount (can be string) in BTC is the value"
16046c16053,16054
<                       }
---
>                       },
>                       "description": "A key-value pair. The key (string) is the bitcoin address,\nthe value (float or string) is the amount in BTC"
```

c94074fa1b

sedited force-pushed on Aug 12, 2026

sedited commented at 1:21 PM on August 12, 2026: contributor

Pushed to address @willcl-ark's comment.

willcl-ark approved

willcl-ark commented at 9:41 AM on August 13, 2026: member

ACK c94074fa1b1396e310ab94955f5d04c9bda61b64

shuv-amp commented at 9:48 AM on August 13, 2026: none

One related schema mismatch may be worth including: ApplyTypeStrOverride describes these arguments as number | string, but all four current users parse the numeric form as an integer. getblockstats, gettxoutsetinfo, and dumptxoutset go through ParseHashOrHeight (getInt<int>), while importdescriptors parses timestamp with getInt<int64_t>. As a result, 0.5 satisfies the emitted schema but all four RPCs reject it with JSON integer out of range.

Using integer | string would describe the accepted numeric form more closely and avoid advertising fractional values to schema consumers. The preference for integer for heights and timestamps was also noted during review of #34683 (comment).

sedited added this to the milestone 32.0 on Aug 14, 2026

willcl-ark commented at 2:21 PM on August 14, 2026: member

Using integer | string would describe the accepted numeric form more closely and avoid advertising fractional values to schema consumers. The preference for integer for heights and timestamps was also noted during review of #34683 (comment).

This seems like a nice/correct tightening to me too. Would be happy to re-ACK this.

sedited commented at 2:29 PM on August 14, 2026: contributor

Pushed to address @shuv-amp's comment. Thanks!

rpc: Fix type in ApplyTypeStrOverride

This should be an integer, not a numeric, as already enforced by the RPC
code and described in the mapping just above the changed line.

e07d826e0e

sedited force-pushed on Aug 14, 2026

DrahtBot added the label CI failed on Aug 14, 2026

willcl-ark approved

willcl-ark commented at 3:35 PM on August 14, 2026: member

ACK e07d826e0ebd9507793fe033236e5f0f12ba5732

willcl-ark commented at 3:36 PM on August 14, 2026: member

I guess the title and/or description could arguably be slightly amended now.

DrahtBot removed the label CI failed on Aug 14, 2026