rpc: Fix named arguments in documentation #18607

maflcko · 2020-04-12T15:42:02Z

This fixes a bug found with #18531:

Currently the named argument for generateblock is documented as address/descriptor, but the server only accepts a named argument of address. Fix it by changing the name to output for both the documentation and the server code. Also, add tests to prove the server understands the new name output.
Unrelated to that, there have been a bunch of aliases in the server code that are not present in the source code of the documentation. Fix that by adding the alias to the source code of the documentation. Only the first alias is displayed in the rendered documentation. Also, add tests to prove the server actually understands all aliases.

DrahtBot · 2020-04-12T22:09:31Z

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

Conflicts

No conflicts as of last run.

pierreN

For what it's worth, tested ACK.

I just left 2 comments below if I were to be extremely picky.

src/rpc/util.cpp

maflcko · 2020-04-16T12:46:57Z

Addressed feedback by @pierreN

maflcko · 2020-04-16T12:59:57Z

And thanks for testing @pierreN . If you want to get included in the merge commit message with your ACK, you can write

Tested ACK ${commit_hash_of_the_commit_you_tested} ${optional_description_of_how and_what_you_tested}

pierreN · 2020-04-16T14:19:44Z

Thanks!

Tested ACK fa168d7 tests, help messages

maflcko · 2020-04-17T16:13:38Z

Rendered diff for reference:

$ git diff --ignore-all-space 
diff --git a/generateblock b/generateblock
index 5586a32..55aca89 100644
--- a/generateblock
+++ b/generateblock
@@ -1,9 +1,9 @@
-generateblock "address/descriptor" ["rawtx/txid",...]
+generateblock "output" ["rawtx/txid",...]
 
 Mine a block with a set of ordered transactions immediately to a specified address or descriptor (before the RPC call returns)
 
 Arguments:
-1. address/descriptor    (string, required) The address or descriptor to send the newly generated bitcoin to.
+1. output               (string, required) The address or descriptor to send the newly generated bitcoin to.
 2. transactions         (json array, required) An array of hex strings which are either txids or raw transactions.
                         Txids must reference transactions currently in the mempool.
                         All transactions must be valid and in valid order, otherwise the block will be rejected.

fa168d7 rpc: Document all aliases for first arg of listtransactions (MarcoFalke) fa5b1f0 rpc: Document all aliases for second arg of getblock (MarcoFalke) fa86a4b rpc: Rename first arg of generateblock RPC to "output" (MarcoFalke) Pull request description: This fixes a bug found with bitcoin#18531: * Currently the named argument for `generateblock` is documented as `address/descriptor`, but the server only accepts a named argument of `address`. Fix it by changing the name to `output` for both the documentation and the server code. Also, add tests to prove the server understands the new name `output`. * Unrelated to that, there have been a bunch of aliases in the server code that are not present in the source code of the documentation. Fix that by adding the alias to the source code of the documentation. Only the first alias is displayed in the rendered documentation. Also, add tests to prove the server actually understands all aliases. ACKs for top commit: pierreN: Tested ACK fa168d7 tests, help messages Tree-SHA512: 05e15628e3a667b296f3783d20f764b450b959451b5360c7eaf5993156582d47a0f5882330ca2493b851eb46324d504953b90c875bc88a15c9e8c89eb3ef8d92

…ones (server) fa7592b rpc: Update server to use new RPCHelpMan (MarcoFalke) aaaaad5 rpc: Add option to hide RPCArg (MarcoFalke) fa9708f rpc: Assert that passed arg names are equal to hardcoded ones (MarcoFalke) faaeb2b rpc: Add CRPCCommand constructor which takes RPCHelpMan (MarcoFalke) fa8ec00 rpc: Check that left section is not multiline (MarcoFalke) Pull request description: This is split out from #18531 to just touch the RPC methods in server. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: laanwj: ACK fa7592b ryanofsky: Code review ACK fa7592b. Looks great! Just some hidden arg and Check() and comment cleanups since last review Tree-SHA512: e64b6a212f4a3aeedeee47557559bde104d5fd40cdc1746b27eb2f3d4c8885d5e6e4dd287595ea11cdbc6a939654fe103cae765fd505875444d851f0abb11308

…ommand ones (server) fa7592b rpc: Update server to use new RPCHelpMan (MarcoFalke) aaaaad5 rpc: Add option to hide RPCArg (MarcoFalke) fa9708f rpc: Assert that passed arg names are equal to hardcoded ones (MarcoFalke) faaeb2b rpc: Add CRPCCommand constructor which takes RPCHelpMan (MarcoFalke) fa8ec00 rpc: Check that left section is not multiline (MarcoFalke) Pull request description: This is split out from bitcoin#18531 to just touch the RPC methods in server. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue bitcoin#18607 * The changes itself fixed bug bitcoin#19250 ACKs for top commit: laanwj: ACK fa7592b ryanofsky: Code review ACK fa7592b. Looks great! Just some hidden arg and Check() and comment cleanups since last review Tree-SHA512: e64b6a212f4a3aeedeee47557559bde104d5fd40cdc1746b27eb2f3d4c8885d5e6e4dd287595ea11cdbc6a939654fe103cae765fd505875444d851f0abb11308

…ones (misc) fa77de2 rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc) (MarcoFalke) fa50bdc rpc: Limit echo to 10 args (MarcoFalke) fa89ca9 refactor: Use C++11 range based for loops to simplify rpc code (MarcoFalke) fa459bd rpc: Treat all args after a hidden arg as hidden as well (MarcoFalke) Pull request description: This is split out from #18531 to just touch the RPC methods in misc. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: laanwj: Code review ACK fa77de2 fjahr: tested ACK fa77de2 theStack: ACK fa77de2 ryanofsky: Code review ACK fa77de2. Pretty straightfoward changes Tree-SHA512: badae1606518c0b55ce2c0bb9025d14f05556532375eb20fd6f3bfadae1e5e6568860bff8599d037e655bf1d23f1f464ca17f4db10a6ab3d502b6e9e61c7b3d3

…ommand ones (misc) fa77de2 rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc) (MarcoFalke) fa50bdc rpc: Limit echo to 10 args (MarcoFalke) fa89ca9 refactor: Use C++11 range based for loops to simplify rpc code (MarcoFalke) fa459bd rpc: Treat all args after a hidden arg as hidden as well (MarcoFalke) Pull request description: This is split out from bitcoin#18531 to just touch the RPC methods in misc. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue bitcoin#18607 * The changes itself fixed bug bitcoin#19250 ACKs for top commit: laanwj: Code review ACK fa77de2 fjahr: tested ACK fa77de2 theStack: ACK bitcoin@fa77de2 ryanofsky: Code review ACK fa77de2. Pretty straightfoward changes Tree-SHA512: badae1606518c0b55ce2c0bb9025d14f05556532375eb20fd6f3bfadae1e5e6568860bff8599d037e655bf1d23f1f464ca17f4db10a6ab3d502b6e9e61c7b3d3

…ommand ones (server) fa7592b rpc: Update server to use new RPCHelpMan (MarcoFalke) aaaaad5 rpc: Add option to hide RPCArg (MarcoFalke) fa9708f rpc: Assert that passed arg names are equal to hardcoded ones (MarcoFalke) faaeb2b rpc: Add CRPCCommand constructor which takes RPCHelpMan (MarcoFalke) fa8ec00 rpc: Check that left section is not multiline (MarcoFalke) Pull request description: This is split out from bitcoin#18531 to just touch the RPC methods in server. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue bitcoin#18607 * The changes itself fixed bug bitcoin#19250 ACKs for top commit: laanwj: ACK fa7592b ryanofsky: Code review ACK fa7592b. Looks great! Just some hidden arg and Check() and comment cleanups since last review Tree-SHA512: e64b6a212f4a3aeedeee47557559bde104d5fd40cdc1746b27eb2f3d4c8885d5e6e4dd287595ea11cdbc6a939654fe103cae765fd505875444d851f0abb11308

…ommand ones (mining,zmq,rpcdump) fa3d9ce rpc: Assert that RPCArg names are equal to CRPCCommand ones (rpcdump) (MarcoFalke) fa32c1d rpc: Assert that RPCArg names are equal to CRPCCommand ones (zmq) (MarcoFalke) faaa46d rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining) (MarcoFalke) fa93bc1 rpc: Remove unused return type from appendCommand (MarcoFalke) Pull request description: This is split out from bitcoin#18531 to just touch the RPC methods in misc. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue bitcoin#18607 * The changes itself fixed bug bitcoin#19250 ACKs for top commit: fjahr: tested ACK fa3d9ce promag: Code review ACK fa3d9ce. Tree-SHA512: 068ade4b55cc195868d53b7f9a27151d45b440857bb069e261a49d102a49a38fdba5d68868516a1d66a54a73ba34681362f934ded7349e894042bde873b75719

…(blockchain,rawtransaction) fa6bb0c Assert that RPCArg names are equal to CRPCCommand ones (rawtransaction) (MarcoFalke) fa80c81 Assert that RPCArg names are equal to CRPCCommand ones (blockchain) (MarcoFalke) Pull request description: This is split out from #18531 to just touch some RPC methods. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: utACK fa6bb0c tryphe: utACK fa6bb0c. Reducing data duplication is nice. Code changes are minimal and concise. Tree-SHA512: deb0edc3f999baf055526eaa199b98c500635e12502dece7aa3cad5319db330eb5ee7459a5c8f040a83671a7f20c560c19a2026fb76c8416f138aa332727cbce

…d ones (blockchain,rawtransaction) fa6bb0c Assert that RPCArg names are equal to CRPCCommand ones (rawtransaction) (MarcoFalke) fa80c81 Assert that RPCArg names are equal to CRPCCommand ones (blockchain) (MarcoFalke) Pull request description: This is split out from bitcoin#18531 to just touch some RPC methods. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue bitcoin#18607 * The changes itself fixed bug bitcoin#19250 ACKs for top commit: fjahr: utACK fa6bb0c tryphe: utACK fa6bb0c. Reducing data duplication is nice. Code changes are minimal and concise. Tree-SHA512: deb0edc3f999baf055526eaa199b98c500635e12502dece7aa3cad5319db330eb5ee7459a5c8f040a83671a7f20c560c19a2026fb76c8416f138aa332727cbce

…(net, rpcwallet) fa14f57 Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) (MarcoFalke) Pull request description: This is the last part split out from #18531 to just touch some RPC methods. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: tACK fa14f57 ryanofsky: Code review ACK fa14f57. Just straightforward replacements except code moved in `addnode`, and displatching updated in `bumpfee_helper` Tree-SHA512: e07af150f1d95a88e558256ce197a6b7dc6cd722a6d6c13c75d944c49c2e2441f8b8237e9f94b03db69fa18f9bda627b0781d5e1da70bf5415e09b38728a8cb1

…d ones (net, rpcwallet) fa14f57 Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) (MarcoFalke) Pull request description: This is the last part split out from bitcoin#18531 to just touch some RPC methods. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue bitcoin#18607 * The changes itself fixed bug bitcoin#19250 ACKs for top commit: fjahr: tACK fa14f57 ryanofsky: Code review ACK fa14f57. Just straightforward replacements except code moved in `addnode`, and displatching updated in `bumpfee_helper` Tree-SHA512: e07af150f1d95a88e558256ce197a6b7dc6cd722a6d6c13c75d944c49c2e2441f8b8237e9f94b03db69fa18f9bda627b0781d5e1da70bf5415e09b38728a8cb1

faaf9c5 remove CRPCCommand constructor that takes rpcfn_type function pointer (MarcoFalke) fa19bb2 remove dead rpc code (MarcoFalke) Pull request description: Remove the CRPCCommand arguments, now that they are asserted to be equal and thus redundant ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: tested ACK faaf9c5 promag: Tested ACK faaf9c5. ryanofsky: Code review ACK faaf9c5. Two obviously good simplifications. Tree-SHA512: 5de3b440f7b2ed2c3e86655d4f0e2e5df9c67e8ce3c7817d5ea5311d1a38690f2f3e28fab41aad6936be9fc884326d037e5f19e85d4d2fe281474dada13911ee

faaf9c5 remove CRPCCommand constructor that takes rpcfn_type function pointer (MarcoFalke) fa19bb2 remove dead rpc code (MarcoFalke) Pull request description: Remove the CRPCCommand arguments, now that they are asserted to be equal and thus redundant ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue bitcoin#18607 * The changes itself fixed bug bitcoin#19250 ACKs for top commit: fjahr: tested ACK faaf9c5 promag: Tested ACK faaf9c5. ryanofsky: Code review ACK faaf9c5. Two obviously good simplifications. Tree-SHA512: 5de3b440f7b2ed2c3e86655d4f0e2e5df9c67e8ce3c7817d5ea5311d1a38690f2f3e28fab41aad6936be9fc884326d037e5f19e85d4d2fe281474dada13911ee

Summary: ``` This fixes a bug found with #18531: Currently the named argument for generateblock is documented as address/descriptor, but the server only accepts a named argument of address. Fix it by changing the name to output for both the documentation and the server code. Also, add tests to prove the server understands the new name output. Unrelated to that, there have been a bunch of aliases in the server code that are not present in the source code of the documentation. Fix that by adding the alias to the source code of the documentation. Only the first alias is displayed in the rendered documentation. Also, add tests to prove the server actually understands all aliases. ``` Backport of core [[bitcoin/bitcoin#18607 | PR18607]]. Depends on D8567. Test Plan: ninja all check-all Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D8568

maflcko force-pushed the 2004-rpcDoc branch from fa90dcb to fa3d5ce Compare April 12, 2020 15:52

DrahtBot added Mining RPC/REST/ZMQ Tests labels Apr 12, 2020

rpc: Rename first arg of generateblock RPC to "output"

fa86a4b

maflcko force-pushed the 2004-rpcDoc branch from fa3d5ce to fa8a3e0 Compare April 12, 2020 21:05

maflcko removed the Mining label Apr 12, 2020

maflcko mentioned this pull request Apr 12, 2020

rpc: remove deprecated CRPCCommand constructor #18531

Merged

maflcko removed the Tests label Apr 13, 2020

pierreN approved these changes Apr 15, 2020

View reviewed changes

src/rpc/util.cpp Show resolved Hide resolved

src/rpc/util.cpp Outdated Show resolved Hide resolved

maflcko force-pushed the 2004-rpcDoc branch from fa8a3e0 to faef2bc Compare April 16, 2020 12:42

MarcoFalke added 2 commits April 16, 2020 08:45

rpc: Document all aliases for second arg of getblock

fa5b1f0

rpc: Document all aliases for first arg of listtransactions

fa168d7

maflcko force-pushed the 2004-rpcDoc branch from faef2bc to fa168d7 Compare April 16, 2020 12:46

maflcko merged commit 244daa4 into bitcoin:master Apr 17, 2020

maflcko deleted the 2004-rpcDoc branch April 17, 2020 16:53

maflcko mentioned this pull request Jun 26, 2020

rpc: Assert that RPCArg names are equal to CRPCCommand ones (server) #19386

Merged

maflcko mentioned this pull request Jul 15, 2020

rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc) #19528

Merged

maflcko mentioned this pull request Aug 14, 2020

rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining,zmq,rpcdump) #19717

Merged

maflcko mentioned this pull request Aug 31, 2020

Assert that RPCArg names are equal to CRPCCommand ones (blockchain,rawtransaction) #19849

Merged

maflcko mentioned this pull request Sep 22, 2020

Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) #19994

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

rpc: Fix named arguments in documentation #18607

rpc: Fix named arguments in documentation #18607

Uh oh!

maflcko commented Apr 12, 2020 •

edited

Loading

Uh oh!

DrahtBot commented Apr 12, 2020 •

edited

Loading

Uh oh!

pierreN left a comment

Uh oh!

Uh oh!

Uh oh!

maflcko commented Apr 16, 2020

Uh oh!

maflcko commented Apr 16, 2020

Uh oh!

pierreN commented Apr 16, 2020

Uh oh!

maflcko commented Apr 17, 2020

Uh oh!

Uh oh!

rpc: Fix named arguments in documentation #18607

rpc: Fix named arguments in documentation #18607

Uh oh!

Conversation

maflcko commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

pierreN left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maflcko commented Apr 16, 2020

Uh oh!

maflcko commented Apr 16, 2020

Uh oh!

pierreN commented Apr 16, 2020

Uh oh!

maflcko commented Apr 17, 2020

Uh oh!

Uh oh!

maflcko commented Apr 12, 2020 •

edited

Loading

DrahtBot commented Apr 12, 2020 •

edited

Loading