Skip to content

Commit

Permalink
Merge bitcoin#16240: JSONRPCRequest-aware RPCHelpMan
Browse files Browse the repository at this point in the history
b6fb617 rpc: switch to using RPCHelpMan.Check() (Karl-Johan Alm)
c7a9fc2 Make the RPCHelpMan aware of JSONRPCRequest and add Check() helper (Karl-Johan Alm)
5c5e32b rpc: migrate JSONRPCRequest functionality into request.cpp (Karl-Johan Alm)
0ab8ba1 rpc: fix RPC help requirements for getblocktemplate (Karl-Johan Alm)

Pull request description:

  Every single RPC call has a helper-section at the start, which throws a help string if the user asks for help or if the user provided too few/many arguments.

  ```C++
  const RPCHelpMan help{...};
  if (request.fHelp || !help.IsValidNumArgs(request.params.size())) {
      throw std::runtime_error(help.ToString());
  }
  ```

  or (older version)

  ```C++
  if (request.fHelp || request.params.size() < min || request.params.size() > max)
      throw std::runtime_error(
          RPCHelpMan{...}.ToString()
      );
  ```

  It seems like an obvious improvement, and less copy-pasting, to make `RPCHelpMan` aware of `JSONRPCRequest`, and to let it handle the checks instead. Both of the above become

  ```C++
  RPCHelpMan{...}.Check(request);
  ```

  which means we save roughly 3 lines per RPC command, and the `RPCHelpMan` instance is never referenced afterwards, so the approach is a tiny fraction cleaner.

  This is a complete update, sans a few special case locations that had special rules. 623 lines turn into 284 (which includes the addition to `RPCHelpMan`).

ACKs for top commit:
  laanwj:
    code rview and lightly tested ACK b6fb617
  MarcoFalke:
    ACK b6fb617, looked at the diff, verified move-only where applicable

Tree-SHA512: eb73f47f812512905b852e313281d1c8df803db40a6188aa39d5a7586631664db6764491152a8a96769946c796dc56d38c6e3a66ddd06ba3fb9d20050e6274e1
  • Loading branch information
MarcoFalke authored and vijaydasmp committed Jan 25, 2022
1 parent 3066042 commit 02a0289
Show file tree
Hide file tree
Showing 18 changed files with 235 additions and 529 deletions.
5 changes: 3 additions & 2 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,10 @@ BITCOIN_CORE_H = \
rpc/client.h \
rpc/mining.h \
rpc/protocol.h \
rpc/server.h \
rpc/rawtransaction_util.h \
rpc/register.h \
rpc/request.h \
rpc/server.h \
rpc/util.h \
saltedhasher.h \
scheduler.h \
Expand Down Expand Up @@ -662,7 +663,7 @@ libdash_util_a_SOURCES = \
interfaces/handler.cpp \
logging.cpp \
random.cpp \
rpc/protocol.cpp \
rpc/request.cpp \
stacktraces.cpp \
support/cleanse.cpp \
sync.cpp \
Expand Down
1 change: 1 addition & 0 deletions src/dash-cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <rpc/client.h>
#include <rpc/protocol.h>
#include <stacktraces.h>
#include <rpc/request.h>
#include <util/system.h>
#include <util/strencodings.h>

Expand Down
1 change: 1 addition & 0 deletions src/rest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <primitives/block.h>
#include <primitives/transaction.h>
#include <rpc/blockchain.h>
#include <rpc/protocol.h>
#include <rpc/server.h>
#include <streams.h>
#include <sync.h>
Expand Down
Loading

0 comments on commit 02a0289

Please sign in to comment.