-
Notifications
You must be signed in to change notification settings - Fork 38.7k
rpc: RPCHelpMan fixes #21913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rpc: RPCHelpMan fixes #21913
Conversation
It is not a dictionary with the single key 'msg'.
|
Concept ACK. I wasn't aware of these (new?) RPCHelpMan methods. |
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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,
There was a problem hiding this comment.
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.
92709d0 to
4983f4c
Compare
promag
left a comment
There was a problem hiding this 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.
|
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 |
|
Documentation diff ACK 6e2eb0d |
|
@MarcoFalke can you have a look here? |
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
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.