rpc: Fix named arguments in documentation #18607

pull MarcoFalke wants to merge 3 commits into bitcoin:master from MarcoFalke:2004-rpcDoc changing 7 files +48 −21
  1. MarcoFalke commented at 3:42 PM on April 12, 2020: member

    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.

  2. MarcoFalke force-pushed on Apr 12, 2020
  3. DrahtBot added the label Mining on Apr 12, 2020
  4. DrahtBot added the label RPC/REST/ZMQ on Apr 12, 2020
  5. DrahtBot added the label Tests on Apr 12, 2020
  6. rpc: Rename first arg of generateblock RPC to "output" fa86a4bbfc
  7. MarcoFalke force-pushed on Apr 12, 2020
  8. MarcoFalke removed the label Mining on Apr 12, 2020
  9. DrahtBot commented at 10:09 PM on April 12, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  10. MarcoFalke cross-referenced this on Apr 12, 2020 from issue rpc: remove deprecated CRPCCommand constructor by MarcoFalke
  11. MarcoFalke removed the label Tests on Apr 13, 2020
  12. in src/rpc/util.cpp:428 in fa8a3e04a8 outdated 
     421 | @@ -419,8 +422,12 @@ RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector<RP
 422 |  {
 423 |      std::set<std::string> named_args;
 424 |      for (const auto& arg : m_args) {
 425 | +        std::vector<std::string> names;
 426 | +        boost::split(names, arg.m_names, boost::is_any_of("|"));
 427 |          // Should have unique named arguments
 428 | -        CHECK_NONFATAL(named_args.insert(arg.m_name).second);
 429 | +        for (const std::string& name : names) {
    pierreN commented at 9:20 PM on April 15, 2020:

    Since you create accessors for m_names (GetName, GetFirstName), it would make sense to make m_names private, no?

    Plus this would allow you to have a function GetNames similar to:

    std::vector<std::string> RPCArg::GetNames() const
{
    std::vector<std::string> names;
    boost::split(names, m_names, boost::is_any_of("|"));
    return names;
}

    and do here more cleanly for (const std::string& name : arg.GetNames()) {

    (note that Fallback, m_fallback, m_description, m_oneline_description can also be made private without any additional code change)

    MarcoFalke commented at 12:38 PM on April 16, 2020:

    RPCArg is a struct, because it was meant to be a stupid class to only hold data and not have complicated member functions. I see that with the name aliases this is no longer true. I think it can be made a class and the GetNames utility can be added. Though, I will leave this pull as is for now, because currently GetNames is only used in this one place. And just in-lining it will yield the least amount of LOC written.

  13. in src/rpc/util.cpp:519 in fa8a3e04a8 outdated 
     512 | @@ -506,6 +513,18 @@ std::string RPCHelpMan::ToString() const
 513 |      return ret;
 514 |  }
 515 |  
 516 | +std::string RPCArg::GetFirstName() const
 517 | +{
 518 | +    auto ret = m_names.substr(0, m_names.find("|"));
 519 | +    return ret;
    pierreN commented at 9:23 PM on April 15, 2020:

    For style, I believe this could be a one-liner, no? return m_names.substr(0, m_names.find("|")); (or have a better variable name than ret)

    But this is personal taste (and you know the codebase style better than I do :)

    MarcoFalke commented at 12:46 PM on April 16, 2020:

    Thanks, fixed and force pushed.

  14. pierreN approved
  15. pierreN commented at 9:24 PM on April 15, 2020: contributor

    For what it's worth, tested ACK.

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

  16. MarcoFalke force-pushed on Apr 16, 2020
  17. rpc: Document all aliases for second arg of getblock fa5b1f067f
  18. rpc: Document all aliases for first arg of listtransactions fa168d7542
  19. MarcoFalke force-pushed on Apr 16, 2020
  20. MarcoFalke commented at 12:46 PM on April 16, 2020: member

    Addressed feedback by @pierreN

  21. MarcoFalke commented at 12:59 PM on April 16, 2020: member

    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}

  22. pierreN commented at 2:19 PM on April 16, 2020: contributor

    Thanks!

    Tested ACK fa168d7 tests, help messages

  23. MarcoFalke commented at 4:13 PM on April 17, 2020: member

    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.
  24. MarcoFalke merged this on Apr 17, 2020
  25. MarcoFalke closed this on Apr 17, 2020
  26. sidhujag referenced this in commit 645c5f7960 on Apr 17, 2020
  27. MarcoFalke deleted the branch on Apr 17, 2020
  28. MarcoFalke cross-referenced this on Jun 26, 2020 from issue rpc: Assert that RPCArg names are equal to CRPCCommand ones (server) by MarcoFalke
  29. MarcoFalke referenced this in commit 804ca26629 on Jul 15, 2020
  30. MarcoFalke cross-referenced this on Jul 15, 2020 from issue rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc) by MarcoFalke
  31. sidhujag referenced this in commit ea73ed0a45 on Jul 16, 2020
  32. MarcoFalke referenced this in commit 31760bb7c9 on Aug 14, 2020
  33. MarcoFalke cross-referenced this on Aug 14, 2020 from issue rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining,zmq,rpcdump) by MarcoFalke
  34. sidhujag referenced this in commit b761dae549 on Aug 16, 2020
  35. sidhujag referenced this in commit 010a3815db on Aug 16, 2020
  36. MarcoFalke referenced this in commit 89a8299a14 on Aug 31, 2020
  37. MarcoFalke cross-referenced this on Aug 31, 2020 from issue Assert that RPCArg names are equal to CRPCCommand ones (blockchain,rawtransaction) by MarcoFalke
  38. sidhujag referenced this in commit 4c2ef86224 on Aug 31, 2020
  39. MarcoFalke referenced this in commit d692d192cd on Sep 22, 2020
  40. MarcoFalke cross-referenced this on Sep 22, 2020 from issue Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) by MarcoFalke
  41. sidhujag referenced this in commit f769a0d8b6 on Sep 23, 2020
  42. MarcoFalke referenced this in commit 5e14fafb31 on Sep 23, 2020
  43. sidhujag referenced this in commit 63306f71f5 on Sep 23, 2020
  44. MarcoFalke referenced this in commit 71d068db40 on Nov 19, 2020
  45. sidhujag referenced this in commit 92bd89791a on Nov 19, 2020
  46. jasonbcox referenced this in commit ec1d2d93d8 on Dec 1, 2020
  47. bitcoin locked this on Feb 15, 2022
Contributors
MarcoFalkeDrahtBotpierreN
Labels
RPC/REST/ZMQ
Linked (view graph)
#18531 rpc: remove deprecated CRPCCommand constructor#19386 rpc: Assert that RPCArg names are equal to CRPCCommand ones (server)#19528 rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc)#19717 rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining,zmq,rpcdump)#19849 Assert that RPCArg names are equal to CRPCCommand ones (blockchain,rawtransaction)#19994 Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet)
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC