RPCHelpMan: Check default values are given at compile-time

MarcoFalke commented at 8:40 pm on December 10, 2018: member

Remove the run time assertions on the default values and ensure that the correct default type and value is provided at compile time.

MarcoFalke added the label Refactoring on Dec 10, 2018

MarcoFalke added the label Docs on Dec 10, 2018

MarcoFalke added the label RPC/REST/ZMQ on Dec 10, 2018

MarcoFalke force-pushed on Dec 10, 2018

DrahtBot commented at 1:20 am on December 11, 2018: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15341 (rpc: Support specifying change address in bumpfee by promag)
#15006 (Add option to create an encrypted wallet by achow101)
#14707 ([RPC] Include coinbase transactions in receivedby RPCs by andrewtoth)
#14641 (rpc: Add min_conf option to fund transaction calls by promag)
#14481 (Add P2SH-P2WSH support to listunspent RPC by MeshCollider)
#13541 (wallet/rpc: sendrawtransaction maxfeerate by kallewoof)
#12674 (RPC: Support addnode onetry without making the connection priviliged by luke-jr)
#11484 (Optional update rescan option in importmulti RPC by pedrobranco)
#10593 (Relax punishment for peers relaying invalid blocks and headers by luke-jr)

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.

DrahtBot added the label Needs rebase on Dec 21, 2018

MarcoFalke force-pushed on Dec 21, 2018

DrahtBot removed the label Needs rebase on Dec 21, 2018

MarcoFalke force-pushed on Dec 21, 2018

DrahtBot added the label Needs rebase on Dec 24, 2018

MarcoFalke force-pushed on Dec 24, 2018

DrahtBot removed the label Needs rebase on Dec 24, 2018

MarcoFalke force-pushed on Jan 15, 2019

DrahtBot added the label Needs rebase on Jan 16, 2019

MarcoFalke force-pushed on Jan 16, 2019

DrahtBot removed the label Needs rebase on Jan 16, 2019

DrahtBot added the label Needs rebase on Jan 21, 2019

MarcoFalke force-pushed on Jan 25, 2019

DrahtBot removed the label Needs rebase on Jan 25, 2019

DrahtBot added the label Needs rebase on Jan 29, 2019

MarcoFalke force-pushed on Jan 29, 2019

DrahtBot removed the label Needs rebase on Jan 29, 2019

DrahtBot added the label Needs rebase on Jan 30, 2019

MarcoFalke force-pushed on Feb 6, 2019

DrahtBot removed the label Needs rebase on Feb 6, 2019

MarcoFalke force-pushed on Feb 7, 2019

DrahtBot added the label Needs rebase on Feb 7, 2019

MarcoFalke force-pushed on Feb 8, 2019

DrahtBot removed the label Needs rebase on Feb 8, 2019

DrahtBot added the label Needs rebase on Feb 10, 2019

RPCHelpMan: Check default values are given at compile-time fa0ad4e7ce

MarcoFalke force-pushed on Feb 11, 2019

MarcoFalke commented at 1:49 pm on February 11, 2019: member

@ryanofsky Mind to take a look here? You requested this in #14877#pullrequestreview-183308244

DrahtBot removed the label Needs rebase on Feb 11, 2019

in src/rpc/util.h:49 in fa0ad4e7ce

40@@ -39,27 +41,42 @@ struct RPCArg {
41         AMOUNT,        //!< Special type representing a floating point amount (can be either NUM or STR)
42         STR_HEX,       //!< Special type that is a STR with only hex chars
43     };
44+
45+    enum class Optional {
46+        /** Required arg */
47+        NO,
48+        /**
49+         * Optinal arg that is a named argument and has a default value of

ryanofsky commented at 11:30 pm on February 12, 2019:

spelling optinal

thijstriemstra commented at 1:40 am on February 13, 2019:

yea.. why was this ignored?

ryanofsky commented at 1:47 am on February 13, 2019:

yea.. why was this ignored?

Just in order to able to merge the pr quickly (it was big and needed to be rebased a lot). The spelling could be fixed later.

ryanofsky approved

ryanofsky commented at 11:30 pm on February 12, 2019: member

utACK fa0ad4e7ce8f4c19fe58bf06747bf8c62600581c

@ryanofsky Mind to take a look here? You requested this in #14877 (review)

This is a big monster change, but it does make things clearer. I reviewed the code and also diffed the output using #14726#pullrequestreview-175423309. All the changes looked like improvements.

  0@@ -59,7 +59,7 @@
  1        "key",      (string) bitcoin address or hex-encoded public key
  2        ...
  3      ]
  4-3. label           (string, optional, default=null) A label to assign the addresses to.
  5+3. label           (string, optional) A label to assign the addresses to.
  6 4. address_type    (string, optional, default=set by -addresstype) The address type to use. Options are "legacy", "p2sh-segwit", and "bech32".
  7 
  8 Result:
  9@@ -121,7 +121,7 @@
 10 
 11 Arguments:
 12 1. txid                           (string, required) The txid to be bumped
 13-2. options                        (json object, optional, default=null)
 14+2. options                        (json object, optional)
 15      {
 16        "confTarget": n,           (numeric, optional, default=fallback to wallet's default) Confirmation target (in blocks)
 17        "totalFee": n,             (numeric, optional, default=fallback to 'confTarget') Total fee (NOT feerate) to pay, in satoshis.
 18@@ -782,7 +782,7 @@
 19 
 20 Arguments:
 21 1. hexstring                          (string, required) The hex string of the raw transaction
 22-2. options                            (json object, optional, default=null) for backward compatibility: passing in a true instead of an object will result in {"includeWatching":true}
 23+2. options                            (json object, optional) for backward compatibility: passing in a true instead of an object will result in {"includeWatching":true}
 24      {
 25        "changeAddress": "str",        (string, optional, default=pool address) The bitcoin address to receive the change
 26        "changePosition": n,           (numeric, optional, default=random) The index of the change output
 27@@ -972,7 +972,7 @@
 28 thus affected by options which limit spendability such as -spendzeroconfchange.
 29 
 30 Arguments:
 31-1. dummy                (string, optional, default=null) Remains for backward compatibility. Must be excluded or set to "*".
 32+1. dummy                (string, optional) Remains for backward compatibility. Must be excluded or set to "*".
 33 2. minconf              (numeric, optional, default=0) Only include transactions confirmed at least this many times.
 34 3. include_watchonly    (boolean, optional, default=false) Also include balance in watch-only addresses (see 'importaddress')
 35 
 36@@ -1682,7 +1682,7 @@
 37 so payments received with the address will be associated with 'label'.
 38 
 39 Arguments:
 40-1. label           (string, optional, default=null) The label name for the address to be linked to. If not provided, the default label "" is used. It can also be set to the empty string "" to represent the default label. The label does not need to exist, it will be created if there is no label by the given name.
 41+1. label           (string, optional, default="") The label name for the address to be linked to. It can also be set to the empty string "" to represent the default label. The label does not need to exist, it will be created if there is no label by the given name.
 42 2. address_type    (string, optional, default=set by -addresstype) The address type to use. Options are "legacy", "p2sh-segwit", and "bech32".
 43 
 44 Result:
 45@@ -1859,7 +1859,7 @@
 46 Arguments:
 47 1. txid         (string, required) The transaction id
 48 2. verbose      (boolean, optional, default=false) If false, return a string, otherwise return a json object
 49-3. blockhash    (string, optional, default=null) The block in which to look for the transaction
 50+3. blockhash    (string, optional) The block in which to look for the transaction
 51 
 52 Result (if verbose is not set or set to false):
 53 "data"      (string) The serialized, hex-encoded data for 'txid'
 54@@ -2080,7 +2080,7 @@
 55        "txid",    (string) A transaction hash
 56        ...
 57      ]
 58-2. blockhash      (string, optional, default=null) If specified, looks for txid in the block with this hash
 59+2. blockhash      (string, optional) If specified, looks for txid in the block with this hash
 60 
 61 Result:
 62 "data"           (string) A string that is a serialized, hex-encoded data for the proof.
 63@@ -2191,7 +2191,7 @@
 64 1. requests                                                         (json array, required) Data to be imported
 65      [
 66        {                                                            (json object)
 67-         "desc": "str",                                             (string, optional) Descriptor to import. If using descriptor, do not also provide address/scriptPubKey, scripts, or pubkeys
 68+         "desc": "str",                                             (string) Descriptor to import. If using descriptor, do not also provide address/scriptPubKey, scripts, or pubkeys
 69          "scriptPubKey": "<script>" | { "address":"<address>" },    (string / json, required) Type of scriptPubKey (string for script, json for address). Should not be provided if using a descriptor
 70          "timestamp": timestamp | "now",                            (integer / string, required) Creation time of the key in seconds since epoch (Jan 1 1970 GMT),
 71                                                                     or the string "now" to substitute the current synced blockchain time. The timestamp of the oldest
 72@@ -2199,8 +2199,8 @@
 73                                                                     "now" can be specified to bypass scanning, for keys which are known to never have been used, and
 74                                                                     0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key
 75                                                                     creation time of all keys being imported by the importmulti call will be scanned.
 76-         "redeemscript": "str",                                     (string, optional, default=omitted) Allowed only if the scriptPubKey is a P2SH or P2SH-P2WSH address/scriptPubKey
 77-         "witnessscript": "str",                                    (string, optional, default=omitted) Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey
 78+         "redeemscript": "str",                                     (string) Allowed only if the scriptPubKey is a P2SH or P2SH-P2WSH address/scriptPubKey
 79+         "witnessscript": "str",                                    (string) Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey
 80          "pubkeys": [                                               (json array, optional, default=empty array) 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).
 81            "pubKey",                                                (string)
 82            ...
 83@@ -2209,7 +2209,7 @@
 84            "key",                                                   (string)
 85            ...
 86          ],
 87-         {                                                          (json object, optional) If a ranged descriptor is used, this specifies the start and end of the range to import
 88+         {                                                          (json object) If a ranged descriptor is used, this specifies the start and end of the range to import
 89            "start": n,                                              (numeric, optional, default=0) Start of the range to import
 90            "end": n,                                                (numeric, required) End of the range to import (inclusive)
 91          },
 92@@ -2219,7 +2219,7 @@
 93        },
 94        ...
 95      ]
 96-2. options                                                          (json object, optional, default=null)
 97+2. options                                                          (json object, optional)
 98      {
 99        "rescan": bool,                                              (boolean, optional, default=true) Stating if should rescan the blockchain after all imports
100      }
101@@ -2379,7 +2379,7 @@
102 Returns the list of all labels, or labels that are assigned to addresses with a specific purpose.
103 
104 Arguments:
105-1. purpose    (string, optional, default=null) Address purpose to list labels for ('send','receive'). An empty string is the same as not providing this argument.
106+1. purpose    (string, optional) Address purpose to list labels for ('send','receive'). An empty string is the same as not providing this argument.
107 
108 Result:
109 [               (json array of string)
110@@ -2442,7 +2442,7 @@
111 1. minconf              (numeric, optional, default=1) The minimum number of confirmations before payments are included.
112 2. include_empty        (boolean, optional, default=false) Whether to include addresses that haven't received any payments.
113 3. include_watchonly    (boolean, optional, default=false) Whether to include watch-only addresses (see 'importaddress').
114-4. address_filter       (string, optional, default=null) If present, only return information on this address.
115+4. address_filter       (string, optional) If present, only return information on this address.
116 
117 Result:
118 [
119@@ -2500,7 +2500,7 @@
120 Additionally, if include_removed is set, transactions affecting the wallet which were removed are returned in the "removed" array.
121 
122 Arguments:
123-1. blockhash               (string, optional, default=null) If set, the block hash to list transactions since, otherwise list all transactions.
124+1. blockhash               (string, optional) If set, the block hash to list transactions since, otherwise list all transactions.
125 2. target_confirmations    (numeric, optional, default=1) Return the nth block hash from the main chain. e.g. 1 would mean the best block hash. Note: this is not used as a filter, but only affects [lastblock] in the return value
126 3. include_watchonly       (boolean, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')
127 4. include_removed         (boolean, optional, default=true) Show transactions that were removed due to a reorg in the "removed" array
128@@ -2555,7 +2555,7 @@
129 Returns up to 'count' most recent transactions skipping the first 'from' transactions.
130 
131 Arguments:
132-1. label                (string, optional, default=null) If set, should be a valid label name to return only incoming transactions
133+1. label                (string, optional) If set, should be a valid label name to return only incoming transactions
134                         with the specified label, or "*" to disable filtering and return all transactions.
135 2. count                (numeric, optional, default=10) The number of transactions to return
136 3. skip                 (numeric, optional, default=0) The number of transactions to skip
137@@ -2622,7 +2622,7 @@
138      ]
139 4. include_unsafe                     (boolean, optional, default=true) Include outputs that are not safe to spend
140                                       See description of "safe" attribute below.
141-5. query_options                      (json object, optional, default=null) JSON with query options
142+5. query_options                      (json object, optional) JSON with query options
143      {
144        "minimumAmount": amount,       (numeric or string, optional, default=0) Minimum value of each UTXO in BTC
145        "maximumAmount": amount,       (numeric or string, optional, default=unlimited) Maximum value of each UTXO in BTC
146@@ -2766,12 +2766,12 @@
147   - "none", "0" : even if other logging categories are specified, ignore all of them.
148 
149 Arguments:
150-1. include                    (json array, optional, default=null) A json array of categories to add debug logging
151+1. include                    (json array, optional) A json array of categories to add debug logging
152      [
153        "include_category",    (string) the valid logging category
154        ...
155      ]
156-2. exclude                    (json array, optional, default=null) A json array of categories to remove debug logging
157+2. exclude                    (json array, optional) A json array of categories to remove debug logging
158      [
159        "exclude_category",    (string) the valid logging category
160        ...
161@@ -2820,7 +2820,7 @@
162 
163 Arguments:
164 1. txid         (string, required) The transaction id.
165-2. dummy        (numeric, optional, default=null) API-Compatibility for previous API. Must be zero or null.
166+2. dummy        (numeric, optional) API-Compatibility for previous API. Must be zero or null.
167                 DEPRECATED. For forward compatibility use named arguments and omit this parameter.
168 3. fee_delta    (numeric, required) The fee value (in satoshis) to add (or subtract, if negative).
169                 Note, that this value is not a fee rate. It is a value to modify absolute fee of the TX.
170@@ -2975,8 +2975,8 @@
171        "address": amount,    (numeric or string, required) The bitcoin address is the key, the numeric amount (can be string) in BTC is the value
172      }
173 3. minconf                   (numeric, optional, default=1) Only use the balance confirmed at least this many times.
174-4. comment                   (string, optional, default=null) A comment
175-5. subtractfeefrom           (json array, optional, default=null) A json array with addresses.
176+4. comment                   (string, optional) A comment
177+5. subtractfeefrom           (json array, optional) A json array with addresses.
178                              The fee will be equally deducted from the amount of each selected address.
179                              Those recipients will receive less bitcoins than you enter in their corresponding amount field.
180                              If no addresses are specified here, the sender pays the fee.
181@@ -3044,9 +3044,9 @@
182 Arguments:
183 1. address                  (string, required) The bitcoin address to send to.
184 2. amount                   (numeric or string, required) The amount in BTC to send. eg 0.1
185-3. comment                  (string, optional, default=null) A comment used to store what the transaction is for.
186+3. comment                  (string, optional) A comment used to store what the transaction is for.
187                             This is not part of the transaction, just kept in your wallet.
188-4. comment_to               (string, optional, default=null) A comment to store the name of the person or organization
189+4. comment_to               (string, optional) A comment to store the name of the person or organization
190                             to which you're sending the transaction. This is not part of the 
191                             transaction, just kept in your wallet.
192 5. subtractfeefromamount    (boolean, optional, default=false) The fee will be deducted from the amount being sent.
193@@ -3217,13 +3217,13 @@
194        "privatekey",               (string) private key in base58-encoding
195        ...
196      ]
197-3. prevtxs                         (json array, optional, default=null) A json array of previous dependent transaction outputs
198+3. prevtxs                         (json array, optional) A json array of previous dependent transaction outputs
199      [
200        {                           (json object)
201          "txid": "hex",            (string, required) The transaction id
202          "vout": n,                (numeric, required) The output number
203          "scriptPubKey": "hex",    (string, required) script key
204-         "redeemScript": "hex",    (string, optional, default=omitted) (required for P2SH or P2WSH) redeem script
205+         "redeemScript": "hex",    (string) (required for P2SH or P2WSH) redeem script
206          "amount": amount,         (numeric or string, required) The amount spent
207        },
208        ...
209@@ -3266,13 +3266,13 @@
210 
211 Arguments:
212 1. hexstring                       (string, required) The transaction hex string
213-2. prevtxs                         (json array, optional, default=null) A json array of previous dependent transaction outputs
214+2. prevtxs                         (json array, optional) A json array of previous dependent transaction outputs
215      [
216        {                           (json object)
217          "txid": "hex",            (string, required) The transaction id
218          "vout": n,                (numeric, required) The output number
219          "scriptPubKey": "hex",    (string, required) script key
220-         "redeemScript": "hex",    (string, optional, default=omitted) (required for P2SH or P2WSH)
221+         "redeemScript": "hex",    (string) (required for P2SH or P2WSH) redeem script
222          "amount": amount,         (numeric or string, required) The amount spent
223        },
224        ...
225@@ -3586,7 +3586,7 @@
226        ...
227      ]
228 3. locktime                           (numeric, optional, default=0) Raw locktime. Non-0 value also locktime-activates inputs
229-4. options                            (json object, optional, default=null)
230+4. options                            (json object, optional)
231      {
232        "changeAddress": "hex",        (string, optional, default=pool address) The bitcoin address to receive the change
233        "changePosition": n,           (numeric, optional, default=random) The index of the change output

MarcoFalke referenced this in commit 0d1160e421 on Feb 12, 2019

MarcoFalke merged this on Feb 12, 2019

MarcoFalke closed this on Feb 12, 2019

MarcoFalke deleted the branch on Feb 13, 2019

deadalnix referenced this in commit 20bb047dd3 on May 22, 2020

dzutto referenced this in commit e8968b6528 on Oct 26, 2021

dzutto referenced this in commit 212f81423f on Oct 26, 2021

dzutto referenced this in commit 89c57146cc on Nov 5, 2021

dzutto referenced this in commit 2ff928c850 on Nov 9, 2021

PastaPastaPasta referenced this in commit 4bf6dc07d4 on Nov 13, 2021

pravblockc referenced this in commit 27390e7c49 on Nov 18, 2021

DrahtBot locked this on Dec 16, 2021

RPCHelpMan: Check default values are given at compile-time #14918

Conflicts