getopenrpcinfo emits two defaults that do not satisfy their schemas.
This changes getdeploymentinfo.blockhash to a default hint, since its fallback describes the current chain tip, and makes send.options.include_watching default to boolean false.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36297.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Added assertions for both fields in rpc_openrpc.py, although probably not strictly necessary and they could perhaps be dropped from here...
Concept ACK
Yes, please drop the test (we don't want to check special fields in a general test), and squash the two trivial (style?) fixups . I'll post the rendered diff instead.
@maflcko done.
rendered diff, as promised:
diff --git a/getdeploymentinfo b/getdeploymentinfo
index e827974..2de1a0c 100644
--- a/getdeploymentinfo
+++ b/getdeploymentinfo
@@ -4,7 +4,7 @@ Returns an object containing various state info regarding deployments of consens
Consensus changes for which the new rules are enforced from genesis are not listed in "deployments".
Arguments:
-1. blockhash (string, optional, default="hash of current chain tip") The block hash at which to query deployment state
+1. blockhash (string, optional, default=hash of current chain tip) The block hash at which to query deployment state
Result:
{ (json object)
diff --git a/getopenrpcinfo.full.json b/getopenrpcinfo.full.json
index 6e007c8..0918146 100644
--- a/getopenrpcinfo.full.json
+++ b/getopenrpcinfo.full.json
@@ -5844,7 +5844,7 @@
"schema": {
"type": "string",
"pattern": "^[0-9a-fA-F]+$",
- "default": "hash of current chain tip"
+ "x-bitcoin-default-hint": "hash of current chain tip"
},
"description": "The block hash at which to query deployment state"
}
@@ -12811,7 +12811,7 @@
},
"include_watching": {
"type": "boolean",
- "default": "false",
+ "default": false,
"description": "(DEPRECATED) No longer used"
},
"inputs": {
diff --git a/send b/send
index 7586a57..a0ac488 100644
--- a/send
+++ b/send
@@ -43,7 +43,7 @@ change_address (string, optional, default=automatic) The bitcoin a
change_position (numeric, optional, default=random) The index of the change output
change_type (string, optional, default=set by -changetype) The output type to use. Only valid if change_address is not specified. Options are "legacy", "p2sh-segwit", "bech32", "bech32m".
fee_rate (numeric or string, optional, default=not set, fall back to wallet fee estimation) Specify a fee rate in sat/vB.
-include_watching (boolean, optional, default="false") (DEPRECATED) No longer used
+include_watching (boolean, optional, default=false) (DEPRECATED) No longer used
inputs (json array, optional, default=[]) Specify inputs instead of adding them automatically.
[
{ (json object)
review ACK e35443488db04cf713978f6df5ef72ed2374303a š
<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 e35443488db04cf713978f6df5ef72ed2374303a š
3pk1YEGM8F7MttNjBeLlnJoFZPgpHVA1XvbpDUlJ8UgU1dd/SB+UfBuZRmldnO+yS7OsPuARWip6H+wO5tINAA==
</details>
I did a git grep 'Default{"[^"]* ' and seems like there are a few more that could be changed to default hint in rpc/rawtransasction.cpp and wallet/rpc/spend.cpp?
I did a
git grep 'Default{"[^"]* 'and seems like there are a few more that could be changed to default hint inrpc/rawtransasction.cppandwallet/rpc/spend.cpp?
interestingā¦
i didnāt search like that, rather used my rust generator to build client types and see where it complained. iām AFK now but curious as to why it didnāt complain on those too.
Correct six argument metadata entries that produce misleading or invalid
OpenRPC defaults.
The getdeploymentinfo blockhash fallback and four sighashtype fallbacks
describe how omitted arguments are resolved. They are not literal values
accepted by the RPCs, so mark them as DefaultHint values.
The send include_watching option is boolean, but its string default makes
the generated schema internally inconsistent. Use a boolean value,
matching the analogous sendall option.
Runtime behavior is unchanged.
Ah I see, my generator did not flag them because the generated schemas only constrain these arguments to type: string, so the prose defaults satisfy the schema. The other two defaults failed a hex pattern and a boolean type check.
Included these four more cases in latest push.
ACK b7f740c594ba1482344a7906b799fb62c533464f
ACK b7f740c594ba1482344a7906b799fb62c533464f
Milestone
32.0