Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
…itcoin#19849, bitcoin#19994 - assert for RPCArg names

c503168 fix: rename arguments for 'voteraw' (Konstantin Akimov)
3621966 feat: add todo to drop Throw() from rpc/util.h (Konstantin Akimov)
b54f03a fix: wrong name of argument for coinjoin (Konstantin Akimov)
d016354 refactor: use new format CPCCommand for rpc/coinjoin (Konstantin Akimov)
0e1a311 Merge bitcoin#19994: Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) (MarcoFalke)
af9eb81 fix: wrong name of arguments for RPC (Konstantin Akimov)
c30c8f2 Merge bitcoin#19849: Assert that RPCArg names are equal to CRPCCommand ones (blockchain,rawtransaction) (MarcoFalke)
f525f57 fix: follow-up missing changes from Merge bitcoin#18607: rpc: Fix named arguments in documentation (Konstantin Akimov)
7ac1ee0 Merge bitcoin#19717: rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining,zmq,rpcdump) (MarcoFalke)
860d31f Merge bitcoin#19455: rpc generate: print useful help and error message (Wladimir J. van der Laan)
41c35fd fix: adjust missing arguments and help for misc rpc: debug, echo, mnsync (Konstantin Akimov)
58d923c Merge bitcoin#19528: rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc) (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  This batch of backports asserts that RPCArg names are equal to CRPCCommand ones.

  ## What was done?
  done backports:
   - bitcoin#19994
   - bitcoin#19849
   - bitcoin#19717
   - bitcoin#19455
   - bitcoin#19528

  Beside that same changes are applied for src/coinjoin's rpc.

  There's also applied multiple fixes for various rpcs for cases when RPCArg names are mismatched with CPCCommand

  **Please, note, that this PR is not final fix for all RPCArgs**. There's a lot of dash's rpc that is not refactored that.
  That it is not easy to implement for `quorum command` because the list of arguments (and even their numbers) are different for each sub-command. This fixes are out-of scope of this PR and should be done before bitcoin#18531 is backported.

  See also relevant bitcoin#21035.

  ## How Has This Been Tested?
  I used this helper to see which exactly args are specified wrongly:
  ```cpp
  diff --git a/src/rpc/server.h b/src/rpc/server.h
  index d4a7ba60eb..cdfd741f54 100644
  --- a/src/rpc/server.h
  +++ b/src/rpc/server.h
  @@ -16,6 +16,7 @@
   #include <string>

   #include <univalue.h>
  +#include <logging.h>

   class CRPCCommand;

  @@ -110,6 +111,19 @@ public:
                 fn().GetArgNames(),
                 intptr_t(fn))
       {
  +        if (fn().m_name != name_in || fn().GetArgNames() != args_in) {
  +            std::cerr << "names:  " << fn().m_name << ' ' << name_in << std::endl;
  +            std::cerr << "arg names: " << fn().GetArgNames().size() << std::endl;
  +            for (const auto& i : fn().GetArgNames()) {
  +                std::cerr << "arg: " << i << std::endl;
  +            }
  +            std::cerr << "FIASCO FIASCO FIASCO FIASCO FIASCO FIASCO" << std::endl;
  +        }
           CHECK_NONFATAL(fn().m_name == name_in);
           CHECK_NONFATAL(fn().GetArgNames() == args_in);
       }
  ```

  ## Breaking Changes
  N/A

  Some arguments are renamed in RPC but they have been broken (used incorrect name not same as in docs)

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    (self)utACK c503168

Tree-SHA512: e885a8f8fa8bc282dae092fe8df65a37e2ab6ca559cd0598d54bfc06cacddb3bd6f3c74fa2d9c1551f8a4fbdfdeabb8d065649df66d5809e792aec6f51d0df14
  • Loading branch information
PastaPastaPasta committed Mar 18, 2024
2 parents 857a279 + c503168 commit 1c20180
Show file tree
Hide file tree
Showing 23 changed files with 1,230 additions and 775 deletions.
4 changes: 2 additions & 2 deletions src/rest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ static bool rest_block_notxdetails(const CoreContext& context, HTTPRequest* req,
}

// A bit of a hack - dependency on a function defined in rpc/blockchain.cpp
UniValue getblockchaininfo(const JSONRPCRequest& request);
RPCHelpMan getblockchaininfo();

static bool rest_chaininfo(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart)
{
Expand All @@ -347,7 +347,7 @@ static bool rest_chaininfo(const CoreContext& context, HTTPRequest* req, const s
case RetFormat::JSON: {
JSONRPCRequest jsonRequest(context);
jsonRequest.params = UniValue(UniValue::VARR);
UniValue chainInfoObject = getblockchaininfo(jsonRequest);
UniValue chainInfoObject = getblockchaininfo().HandleRequest(jsonRequest);
std::string strJSON = chainInfoObject.write() + "\n";
req->WriteHeader("Content-Type", "application/json");
req->WriteReply(HTTP_OK, strJSON);
Expand Down
Loading

0 comments on commit 1c20180

Please sign in to comment.