Skip to content

Conversation

@kallewoof
Copy link
Contributor

@kallewoof kallewoof commented May 11, 2021

This is a follow-up to #21897, and I believe covers the remaining cases, at least that I could find.

Edited to remove unrelated information about a side project.

@jonatack
Copy link
Member

Concept ACK. I wasn't aware of these (new?) RPCHelpMan methods.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

top level arguments are always Null if omitted. What makes this one special?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarcoFalke This is the only one without the "optional" keyword in the help:

Arguments:
1. wallet_name             (string, required) The name for the new wallet. If this is a path, the wallet will be created at the path location.
2. disable_private_keys    (boolean, optional, default=false) Disable the possibility of private keys (only watchonlys are possible in this mode).
3. blank                   (boolean, optional, default=false) Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed.
4. passphrase              (string) Encrypt the wallet with this passphrase.
5. avoid_reuse             (boolean, optional, default=false) Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind.
6. descriptors             (boolean, optional, default=false) Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation
7. load_on_startup         (boolean, optional, default=null) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.

Copy link
Contributor Author

@kallewoof kallewoof May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way, this one is not OMITTED which is described as

        /**
         * Optional argument with default value omitted because they are
         * implicitly clear. That is, elements in an array or object may not
         * exist by default.
         * When possible, the default value should be specified.
         */
        OMITTED,

Instead, alternatively, it is OMITTED_NAMED_ARG (its name is "passphrase") or as above, given a null default:

        /**
         * Optional arg that is a named argument and has a default value of
         * `null`. When possible, the default value should be specified.
         */
        OMITTED_NAMED_ARG,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I painted myself into the opinion that this actually is OMITTED_NAMED_ARG so I've pushed that change. Sorry for the spam.

The current output for passphrase results in a non-optional passphrase which is incorrect.
@kallewoof kallewoof force-pushed the 202104-rpchelpman-fixes2 branch from 92709d0 to 4983f4c Compare May 11, 2021 12:04
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 6e2eb0d.

@kallewoof
Copy link
Contributor Author

Rendered diff:

diff --git a/help-output-master.txt b/help-output-21913.txt
index ee49aa730..455e8d455 100644
--- a/help-output-master.txt
+++ b/help-output-21913.txt
@@ -6,7 +6,7 @@ Note this call may take some time if you are not using coinstatsindex.
 
 Arguments:
 1. hash_type         (string, optional, default="hash_serialized_2") Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'muhash', 'none'.
-2. hash_or_height    (string or numeric) The block hash or height of the target height (only available with coinstatsindex).
+2. hash_or_height    (string or numeric, optional) The block hash or height of the target height (only available with coinstatsindex).
 3. use_index         (boolean, optional, default=true) Use coinstatsindex, if available.
 
 Result:
@@ -108,10 +108,11 @@ Result:
       ...
     },
     "bytesrecv_per_msg" : {           (json object)
-      "msg" : n                       (numeric) The total bytes received aggregated by message type
+      "msg" : n,                      (numeric) The total bytes received aggregated by message type
                                       When a message type is not listed in this json object, the bytes received are 0.
                                       Only known message types can appear as keys in the object and all bytes received
                                       of unknown message types are listed under '*other*'.
+      ...
     },
     "connection_type" : "str"         (string) Type of connection: 
                                       outbound-full-relay (default automatic connections),
@@ -131,7 +132,7 @@ Examples:
 > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getpeerinfo", "params": []}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
 
 === createpsbt ===
-createpsbt [{"txid":"hex","vout":n,"sequence":n},...] [{"address":amount},{"data":"hex"},...] ( locktime replaceable )
+createpsbt [{"txid":"hex","vout":n,"sequence":n},...] [{"address":amount,...},{"data":"hex"},...] ( locktime replaceable )
 
 Creates a transaction in the Partially Signed Transaction format.
 Implements the Creator role.
@@ -153,6 +154,7 @@ Arguments:
      [
        {                       (json object)
          "address": amount,    (numeric or string, required) A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in BTC
+         ...
        },
        {                       (json object)
          "data": "hex",        (string, required) A key-value pair. The key must be "data", the value is hex-encoded data
@@ -170,7 +172,7 @@ Examples:
 > bitcoin-cli createpsbt "[{\"txid\":\"myid\",\"vout\":0}]" "[{\"data\":\"00010203\"}]"
 
 === sendmany ===
-sendmany "" {"address":amount} ( minconf "comment" ["address",...] replaceable conf_target "estimate_mode" fee_rate verbose )
+sendmany "" {"address":amount,...} ( minconf "comment" ["address",...] replaceable conf_target "estimate_mode" fee_rate verbose )
 
 Send multiple times. Amounts are double-precision floating point numbers.
 Requires wallet passphrase to be set with walletpassphrase call if wallet is encrypted.
@@ -180,6 +182,7 @@ Arguments:
 2. amounts                   (json object, required) The addresses and amounts
      {
        "address": amount,    (numeric or string, required) The bitcoin address is the key, the numeric amount (can be string) in BTC is the value
+       ...
      }
 3. minconf                   (numeric, optional) Ignored dummy value
 4. comment                   (string, optional) A comment
@@ -234,7 +237,7 @@ applied to the new wallet (eg -rescan, etc).
 
 Arguments:
 1. filename           (string, required) The wallet directory or .dat file.
-2. load_on_startup    (boolean, optional, default=null) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.
+2. load_on_startup    (boolean, optional) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.
 
 Result:
 {                       (json object)
@@ -255,10 +258,10 @@ Arguments:
 1. wallet_name             (string, required) The name for the new wallet. If this is a path, the wallet will be created at the path location.
 2. disable_private_keys    (boolean, optional, default=false) Disable the possibility of private keys (only watchonlys are possible in this mode).
 3. blank                   (boolean, optional, default=false) Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed.
-4. passphrase              (string) Encrypt the wallet with this passphrase.
+4. passphrase              (string, optional) Encrypt the wallet with this passphrase.
 5. avoid_reuse             (boolean, optional, default=false) Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind.
 6. descriptors             (boolean, optional, default=false) Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation
-7. load_on_startup         (boolean, optional, default=null) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.
+7. load_on_startup         (boolean, optional) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.
 8. external_signer         (boolean, optional, default=false) Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true.
 
 Result:
@@ -279,7 +282,7 @@ Unloads the wallet referenced by the request endpoint otherwise unloads the wall
 Specifying the wallet name on a wallet endpoint is invalid.
 Arguments:
 1. wallet_name        (string, optional, default=the wallet name from the RPC endpoint) The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical.
-2. load_on_startup    (boolean, optional, default=null) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.
+2. load_on_startup    (boolean, optional) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.
 
 Result:
 {                       (json object)
@@ -291,7 +294,7 @@ Examples:
 > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "unloadwallet", "params": [wallet_name]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
 
 === send ===
-send [{"address":amount},{"data":"hex"},...] ( conf_target "estimate_mode" fee_rate options )
+send [{"address":amount,...},{"data":"hex"},...] ( conf_target "estimate_mode" fee_rate options )
 
 EXPERIMENTAL warning: this call may be changed in future releases.
 
@@ -304,6 +307,7 @@ Arguments:
      [
        {                                 (json object)
          "address": amount,              (numeric or string, required) A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in BTC
+         ...
        },
        {                                 (json object)
          "data": "hex",                  (string, required) A key-value pair. The key must be "data", the value is hex-encoded data
@@ -378,7 +382,7 @@ Create a transaction that should confirm the next block, with a specific input,
 > bitcoin-cli send '{"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl": 0.1}' 1 economical '{"add_to_wallet": false, "inputs": [{"txid":"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0", "vout":1}]}'
 
 === walletcreatefundedpsbt ===
-walletcreatefundedpsbt ( [{"txid":"hex","vout":n,"sequence":n},...] ) [{"address":amount},{"data":"hex"},...] ( locktime options bip32derivs )
+walletcreatefundedpsbt ( [{"txid":"hex","vout":n,"sequence":n},...] ) [{"address":amount,...},{"data":"hex"},...] ( locktime options bip32derivs )
 
 Creates and funds a transaction in the Partially Signed Transaction format.
 Implements the Creator and Updater roles.
@@ -400,6 +404,7 @@ Arguments:
      [
        {                              (json object)
          "address": amount,           (numeric or string, required) A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in BTC
+         ...
        },
        {                              (json object)
          "data": "hex",               (string, required) A key-value pair. The key must be "data", the value is hex-encoded data

@laanwj
Copy link
Member

laanwj commented May 19, 2021

Documentation diff ACK 6e2eb0d

@fanquake
Copy link
Member

@MarcoFalke can you have a look here?

@maflcko maflcko merged commit ea8b2e8 into bitcoin:master May 20, 2021
@kallewoof kallewoof deleted the 202104-rpchelpman-fixes2 branch May 20, 2021 06:10
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 20, 2021
6e2eb0d rpc/wallet: use OMITTED_NAMED_ARG instead of Default(VNULL) (Karl-Johan Alm)
4983f4c rpc/createwallet: omitted named arguments (Karl-Johan Alm)
dc4db23 rpc: address:amount dictionaries are OBJ_USER_KEYS (Karl-Johan Alm)
c8cf0a3 rpc/getpeerinfo: bytesrecv_per_msg is a dynamic dictionary (Karl-Johan Alm)
eb4fb7e rpc/gettxoutsetinfo: hash_or_height is a named argument (Karl-Johan Alm)

Pull request description:

  This is a follow-up to bitcoin#21897, and I believe covers the remaining cases, at least that I could find.

  Edited to remove unrelated information about a side project.

ACKs for top commit:
  laanwj:
    Documentation diff ACK 6e2eb0d
  promag:
    Code review ACK 6e2eb0d.

Tree-SHA512: d26f6e074e13d64bbca2a114a0adc7f905d47d238c4e9bc49f70ca0b775afbebf9879fc3794ab29dc316a6dbd00ba8cbeb01197e236ee4ab2e9854db25f23f04
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants