rpc: RPCHelpMan: Always name dictionary keys #15746

1. 
   MarcoFalke commented at 2:53 PM on April 4, 2019: member

   Fixes two issues reported in #15737:

   • I am very perplexed as to how the code I'm looking at is generating the help text I'm seeing

   So add documentation

   • This is a value for which a key is missing

   So always serialize the name of the dictionary key if the outer type is a dictionary

2. rpc: Add some doxygen comments to utils fa652b229e
3. rpc: RPCHelpMan: Always push_name when outer type is an object fa26eb5e8f
4. MarcoFalke added the label RPC/REST/ZMQ on Apr 4, 2019
5. MarcoFalke added this to the milestone 0.19.0 on Apr 4, 2019
6. 
   MarcoFalke commented at 3:03 PM on April 4, 2019: member

   This bugfix can not be tested, because none of our RPCs accept nested dictionaries. However, the following diff might help:

   diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
   index 9c5dae3623..81d282ceca 100644
   --- a/src/wallet/rpcdump.cpp
   +++ b/src/wallet/rpcdump.cpp
   @@ -1377,7 +1377,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
                                                {"pubKey", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""},
                                            }
                                        },
   -                                    {"keys", RPCArg::Type::ARR, /* default */ "empty array", "Array of strings giving private keys to import. The corresponding public keys must occur in the output or redeemscript.",
   +                                    {"keys", RPCArg::Type::OBJ, /* default */ "empty array", "Array of strings giving private keys to import. The corresponding public keys must occur in the output or redeemscript.",
                                            {
                                                {"key", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""},
                                            }
   

   With the above diff, before and after my bugfix:

   diff --git a/importmulti b/importmulti
   index 7a79198..d2eb7e7 100644
   --- a/importmulti
   +++ b/importmulti
   @@ -25,7 +25,7 @@ Arguments:
               "pubKey",                                                (string)
               ...
             ],
   -         {                                                          (json object, optional, default=empty array) Array of strings giving private keys to import. The corresponding public keys must occur in the output or redeemscript.
   +         "keys": {                                                  (json object, optional, default=empty array) Array of strings giving private keys to import. The corresponding public keys must occur in the output or redeemscript.
               "key": "str",                                            (string)
             },
             "range": n or [n,n],                                       (numeric or array) If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import
   

7. 
   gwillen commented at 11:08 PM on April 4, 2019: contributor

   Aha, thanks -- this looks exactly like the problem I was seeing with "range", which was a nested dictionary at the time of my last rebase, even though it didn't exist in 0.17, and is a nested array on master.

   Kudos for finding the problem despite the repro being missing. :-)

8. 
   gwillen commented at 11:09 PM on April 4, 2019: contributor

   utACK

9. 
   promag commented at 2:38 PM on April 5, 2019: member

   Tested ACK fa26eb5.

10. in src/rpc/util.cpp:234 in fa26eb5e8f

     230 | @@ -213,7 +231,7 @@ struct Sections {
     231 |          case RPCArg::Type::OBJ:
     232 |          case RPCArg::Type::OBJ_USER_KEYS: {
     233 |              const auto right = outer_type == OuterType::NAMED_ARG ? "" : arg.ToDescriptionString();
     234 | -            PushSection({indent + "{", right});
     235 | +            PushSection({indent + (push_name ? "\"" + arg.m_name + "\": " : "") + "{", right});
    

    ---

    ---

    

    promag commented at 2:42 PM on April 5, 2019:

    nit,

    const auto left = push_name ? "\"" + arg.m_name + "\": " : "";
    PushSection({indent + left + "{", right);
    

11. MarcoFalke merged this on Apr 10, 2019
12. MarcoFalke closed this on Apr 10, 2019

    ---

13. MarcoFalke referenced this in commit 8c022e8ac4 on Apr 10, 2019
14. MarcoFalke deleted the branch on Apr 10, 2019
15. deadalnix referenced this in commit 2429c346e2 on May 22, 2020
16. DrahtBot locked this on Dec 16, 2021

Milestone
0.19.0