OMITTED_NAMED_ARG and OMITTED are a confusing burden:
Fix all issues by making them an alias of each other; Pick the right one based on the outer type.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | fanquake |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, 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.
<details><summary>Rendered RPC doc diff:</summary>
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".
</details>
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)},
^~~~~~~~~~ ~
ACK fad56f7dd6ecac772e1bf590b38cd34ba67db5d7
162 | - OMITTED_NAMED_ARG, 163 | - /** 164 | + * `null`. 165 | + * 166 | * Optional argument with default value omitted because they are 167 | - * implicitly clear. That is, elements in an array or object may not
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
https://github.com/bitcoin/bitcoin/blob/392dc68e37be9fc7adb32496b13d9b50262e317d/src/wallet/rpc/backup.cpp#L1258-L1293
"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)
Thank you, the root of my confusion was the definition of named arguments.