doc: Properly report optional RPC args #26706
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Rendered RPC doc diff:diff --git a/fundrawtransaction b/fundrawtransaction
index a482315..1421b27 100644
--- a/fundrawtransaction
+++ b/fundrawtransaction
@@ -22,7 +22,7 @@ Arguments:
Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.
If that happens, you will need to fund the transaction with different inputs and republish it.
"minconf": n, (numeric, optional, default=0) If add_inputs is specified, require inputs with at least this many confirmations.
- "maxconf": n, (numeric) If add_inputs is specified, require inputs with at most this many confirmations.
+ "maxconf": n, (numeric, optional) If add_inputs is specified, require inputs with at most this many confirmations.
"changeAddress": "str", (string, optional, default=automatic) The bitcoin address to receive the change
"changePosition": n, (numeric, optional, default=random) The index of the change output
"change_type": "str", (string, optional, default=set by -changetype) The output type to use. Only valid if changeAddress is not specified. Options are "legacy", "p2sh-segwit", "bech32", and "bech32m".
diff --git a/importdescriptors b/importdescriptors
index e8d2dd2..44e9ef0 100644
--- a/importdescriptors
+++ b/importdescriptors
@@ -12,8 +12,8 @@ Arguments:
{ (json object)
"desc": "str", (string, required) Descriptor to import.
"active": bool, (boolean, optional, default=false) Set this descriptor to be the active descriptor for the corresponding output type/externality
- "range": n or [n,n], (numeric or array) If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import
- "next_index": n, (numeric) If a ranged descriptor is set to active, this specifies the next index to generate addresses from
+ "range": n or [n,n], (numeric or array, optional) If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import
+ "next_index": n, (numeric, optional) If a ranged descriptor is set to active, this specifies the next index to generate addresses from
"timestamp": timestamp | "now", (integer / string, required) Time from which to start rescanning the blockchain for this descriptor, in UNIX epoch time
Use the string "now" to substitute the current synced blockchain time.
"now" can be specified to bypass scanning, for outputs which are known to never have been used, and
diff --git a/importmulti b/importmulti
index 6433ec3..aa238e2 100644
--- a/importmulti
+++ b/importmulti
@@ -14,7 +14,7 @@ Arguments:
1. requests (json array, required) Data to be imported
[
{ (json object)
- "desc": "str", (string) Descriptor to import. If using descriptor, do not also provide address/scriptPubKey, scripts, or pubkeys
+ "desc": "str", (string, optional) Descriptor to import. If using descriptor, do not also provide address/scriptPubKey, scripts, or pubkeys
"scriptPubKey": "<script>" | { "address":"<address>" }, (string / json, required) Type of scriptPubKey (string for script, json for address). Should not be provided if using a descriptor
"timestamp": timestamp | "now", (integer / string, required) Creation time of the key expressed in UNIX epoch time,
or the string "now" to substitute the current synced blockchain time. The timestamp of the oldest
@@ -22,8 +22,8 @@ Arguments:
"now" can be specified to bypass scanning, for keys which are known to never have been used, and
0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key
creation time of all keys being imported by the importmulti call will be scanned.
- "redeemscript": "str", (string) Allowed only if the scriptPubKey is a P2SH or P2SH-P2WSH address/scriptPubKey
- "witnessscript": "str", (string) Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey
+ "redeemscript": "str", (string, optional) Allowed only if the scriptPubKey is a P2SH or P2SH-P2WSH address/scriptPubKey
+ "witnessscript": "str", (string, optional) Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey
"pubkeys": [ (json array, optional, default=[]) Array of strings giving pubkeys to import. They must occur in P2PKH or P2WPKH scripts. They are not required when the private key is also provided (see the "keys" argument).
"pubKey", (string)
...
@@ -32,7 +32,7 @@ Arguments:
"key", (string)
...
],
- "range": n or [n,n], (numeric or array) If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import
+ "range": n or [n,n], (numeric or array, optional) If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import
"internal": bool, (boolean, optional, default=false) Stating whether matching outputs should be treated as not incoming payments (also known as change)
"watchonly": bool, (boolean, optional, default=false) Stating whether matching outputs should be considered watchonly.
"label": "str", (string, optional, default="") Label to assign to the address, only allowed with internal=false
diff --git a/scanblocks b/scanblocks
index 25d8be5..f1626dc 100644
--- a/scanblocks
+++ b/scanblocks
@@ -8,7 +8,7 @@ Arguments:
"start" for starting a scan
"abort" for aborting the current scan (returns true when abort was successful)
"status" for progress report (in %) of the current scan
-2. scanobjects (json array) Array of scan objects. Required for "start" action
+2. scanobjects (json array, optional) Array of scan objects. Required for "start" action
Every scan object is either a string descriptor or an object:
[
"descriptor", (string) An output descriptor
diff --git a/scantxoutset b/scantxoutset
index 31ed769..654c806 100644
--- a/scantxoutset
+++ b/scantxoutset
@@ -19,7 +19,7 @@ Arguments:
"start" for starting a scan
"abort" for aborting the current scan (returns true when abort was successful)
"status" for progress report (in %) of the current scan
-2. scanobjects (json array) Array of scan objects. Required for "start" action
+2. scanobjects (json array, optional) Array of scan objects. Required for "start" action
Every scan object is either a string descriptor or an object:
[
"descriptor", (string) An output descriptor
diff --git a/send b/send
index 43e53c2..5308b8c 100644
--- a/send
+++ b/send
@@ -32,7 +32,7 @@ Arguments:
Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.
If that happens, you will need to fund the transaction with different inputs and republish it.
"minconf": n, (numeric, optional, default=0) If add_inputs is specified, require inputs with at least this many confirmations.
- "maxconf": n, (numeric) If add_inputs is specified, require inputs with at most this many confirmations.
+ "maxconf": n, (numeric, optional) If add_inputs is specified, require inputs with at most this many confirmations.
"add_to_wallet": bool, (boolean, optional, default=true) When false, returns a serialized transaction which will not be added to the wallet or broadcast
"change_address": "str", (string, optional, default=automatic) The bitcoin address to receive the change
"change_position": n, (numeric, optional, default=random) The index of the change output
diff --git a/sendall b/sendall
index af67fec..8f3e9a7 100644
--- a/sendall
+++ b/sendall
@@ -44,7 +44,7 @@ Arguments:
"psbt": bool, (boolean, optional, default=automatic) Always return a PSBT, implies add_to_wallet=false.
"send_max": bool, (boolean, optional, default=false) When true, only use UTXOs that can pay for their own fees to maximize the output amount. When 'false' (default), no UTXO is left behind. send_max is incompatible with providing specific inputs.
"minconf": n, (numeric, optional, default=0) Require inputs with at least this many confirmations.
- "maxconf": n, (numeric) Require inputs with at most this many confirmations.
+ "maxconf": n, (numeric, optional) Require inputs with at most this many confirmations.
"conf_target": n, (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks
"estimate_mode": "str", (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
"unset"
diff --git a/signrawtransactionwithkey b/signrawtransactionwithkey
index cd163d9..d042181 100644
--- a/signrawtransactionwithkey
+++ b/signrawtransactionwithkey
@@ -19,9 +19,9 @@ Arguments:
"txid": "hex", (string, required) The transaction id
"vout": n, (numeric, required) The output number
"scriptPubKey": "hex", (string, required) script key
- "redeemScript": "hex", (string) (required for P2SH) redeem script
- "witnessScript": "hex", (string) (required for P2WSH or P2SH-P2WSH) witness script
- "amount": amount, (numeric or string) (required for Segwit inputs) the amount spent
+ "redeemScript": "hex", (string, optional) (required for P2SH) redeem script
+ "witnessScript": "hex", (string, optional) (required for P2WSH or P2SH-P2WSH) witness script
+ "amount": amount, (numeric or string, optional) (required for Segwit inputs) the amount spent
},
...
]
diff --git a/signrawtransactionwithwallet b/signrawtransactionwithwallet
index e41e3d0..22d1a2c 100644
--- a/signrawtransactionwithwallet
+++ b/signrawtransactionwithwallet
@@ -13,9 +13,9 @@ Arguments:
"txid": "hex", (string, required) The transaction id
"vout": n, (numeric, required) The output number
"scriptPubKey": "hex", (string, required) script key
- "redeemScript": "hex", (string) (required for P2SH) redeem script
- "witnessScript": "hex", (string) (required for P2WSH or P2SH-P2WSH) witness script
- "amount": amount, (numeric or string) (required for Segwit inputs) the amount spent
+ "redeemScript": "hex", (string, optional) (required for P2SH) redeem script
+ "witnessScript": "hex", (string, optional) (required for P2WSH or P2SH-P2WSH) witness script
+ "amount": amount, (numeric or string, optional) (required for Segwit inputs) the amount spent
},
...
]
diff --git a/walletcreatefundedpsbt b/walletcreatefundedpsbt
index e6ad9d3..511bb43 100644
--- a/walletcreatefundedpsbt
+++ b/walletcreatefundedpsbt
@@ -39,7 +39,7 @@ Arguments:
Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.
If that happens, you will need to fund the transaction with different inputs and republish it.
"minconf": n, (numeric, optional, default=0) If add_inputs is specified, require inputs with at least this many confirmations.
- "maxconf": n, (numeric) If add_inputs is specified, require inputs with at most this many confirmations.
+ "maxconf": n, (numeric, optional) If add_inputs is specified, require inputs with at most this many confirmations.
"changeAddress": "str", (string, optional, default=automatic) The bitcoin address to receive the change
"changePosition": n, (numeric, optional, default=random) The index of the change output
"change_type": "str", (string, optional, default=set by -changetype) The output type to use. Only valid if changeAddress is not specified. Options are "legacy", "p2sh-segwit", "bech32", and "bech32m". |
This was referenced Dec 15, 2022
This was referenced Dec 16, 2022
Merged
|
FWIW, the first commit has been cherry-picked into #26707. |
added 3 commits
January 17, 2023 13:13
…rrors
The warnings look like:
src/rpc/util.h:192:19: error: std::move of the const variable 'name' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg,-warnings-as-errors]
: m_names{std::move(name)},
^~~~~~~~~~ ~
fa9e938 to
fad56f7
Compare
fanquake
approved these changes
Jan 17, 2023
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jan 19, 2023
fad56f7 doc: Properly report optional RPC args (MarcoFalke) fa09cb6 refactor: Introduce is_top_level_arg (MarcoFalke) fab92a5 refactor: Remove const to fix performance-move-const-arg clang-tidy errors (MarcoFalke) Pull request description: `OMITTED_NAMED_ARG` and `OMITTED` are a confusing burden: * It puts the burden on developers to pick the right one of the two * They can be interchanged without introducing a compile failure or other error * Picking the wrong one is leading to incorrect docs * They are redundant, because the correct one can already be determined by the surrounding type Fix all issues by making them an alias of each other; Pick the right one based on the outer type. ACKs for top commit: fanquake: ACK fad56f7 Tree-SHA512: 6e7193a05a852ba1618a9cb3261220c7ad3282bc5595325c04437aa811f529a88e2851e9c7dbf9878389b8aa42e98f8817b7eb0260fbb9e123da0893cbae6ca2
kouloumos
reviewed
Jan 20, 2023
| * Optional argument with default value omitted because they are | ||
| * implicitly clear. That is, elements in an array or object may not |
There was a problem hiding this comment.
Is there a reason that "or object" was removed from here? My understanding is that RPCArg::Optional::OMITTED still applies to elements in objects, such as here
bitcoin/src/wallet/rpc/backup.cpp
Lines 1258 to 1293 in 392dc68
There was a problem hiding this comment.
"or object" is covered by the first of the two reasons:
Optional arg that is a named argument and has a default value of null
(named arguments are basically keys in a dictionary/object)
There was a problem hiding this comment.
Thank you, the root of my confusion was the definition of named arguments.
There was a problem hiding this comment.
If there are improvements, they can be appended to #26919, which is a follow-up
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
OMITTED_NAMED_ARGandOMITTEDare a confusing burden:Fix all issues by making them an alias of each other; Pick the right one based on the outer type.