Skip to content

Commit

Permalink
Drop JSONRPCRequest constructors
Browse files Browse the repository at this point in the history
Summary:
This just makes an additional simplification after D11552 replaced `util::Ref` with `std::any`.

Note that we sometimes wrap the context in a `HTTPRPCRequestProcessor`instance, so some changes don't apply in bitcoind.cpp and init.cpp.

This is a backport of [[bitcoin/bitcoin#21574 | core#21574]]
Depends on D11552

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D11560
  • Loading branch information
ryanofsky authored and PiRK committed Jun 7, 2022
1 parent 67b6f4f commit 5ae5480
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 50 deletions.
3 changes: 2 additions & 1 deletion src/httprpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,8 @@ bool HTTPRPCRequestProcessor::ProcessHTTPRequest(HTTPRequest *req) {
return false;
}

JSONRPCRequest jreq(context);
JSONRPCRequest jreq;
jreq.context = context;
jreq.peerAddr = req->GetPeer().ToString();
if (!RPCAuthorized(authHeader.second, jreq.authUser)) {
LogPrintf("ThreadRPCServer incorrect password attempt from %s\n",
Expand Down
2 changes: 1 addition & 1 deletion src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1523,7 +1523,7 @@ static bool AppInitServers(Config &config,
return false;
}
if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) {
StartREST(httpRPCRequestProcessor.context);
StartREST(&node);
}

StartHTTPServer();
Expand Down
13 changes: 3 additions & 10 deletions src/interfaces/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ namespace {
UniValue executeRpc(const Config &config, const std::string &command,
const UniValue &params,
const std::string &uri) override {
JSONRPCRequest req(m_context_ref);
JSONRPCRequest req;
req.context = m_context;
req.params = params;
req.strMethod = command;
req.URI = uri;
Expand Down Expand Up @@ -332,16 +333,8 @@ namespace {
}));
}
NodeContext *context() override { return m_context; }
void setContext(NodeContext *context) override {
m_context = context;
if (context) {
m_context_ref = context;
} else {
m_context_ref.reset();
}
}
void setContext(NodeContext *context) override { m_context = context; }
NodeContext *m_context{nullptr};
std::any m_context_ref{m_context};
};
} // namespace

Expand Down
6 changes: 4 additions & 2 deletions src/interfaces/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,10 @@ namespace {
[this, &command](const Config &config,
const JSONRPCRequest &request,
UniValue &result, bool last_handler) {
return command.actor(config, {request, &m_context},
result, last_handler);
JSONRPCRequest wallet_request = request;
wallet_request.context = &m_context;
return command.actor(config, wallet_request, result,
last_handler);
},
command.argNames, command.unique_id);
m_rpc_handlers.emplace_back(
Expand Down
7 changes: 4 additions & 3 deletions src/rest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,8 @@ static bool rest_chaininfo(Config &config, const std::any &context,

switch (rf) {
case RetFormat::JSON: {
JSONRPCRequest jsonRequest(context);
JSONRPCRequest jsonRequest;
jsonRequest.context = context;
jsonRequest.params = UniValue(UniValue::VARR);
UniValue chainInfoObject =
getblockchaininfo().HandleRequest(config, jsonRequest);
Expand Down Expand Up @@ -787,8 +788,8 @@ static const struct {

void StartREST(const std::any &context) {
for (const auto &up : uri_prefixes) {
auto handler = [&context, up](Config &config, HTTPRequest *req,
const std::string &prefix) {
auto handler = [context, up](Config &config, HTTPRequest *req,
const std::string &prefix) {
return up.handler(config, context, req, prefix);
};
RegisterHTTPHandler(up.prefix, false, handler);
Expand Down
13 changes: 1 addition & 12 deletions src/rpc/request.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,7 @@ class JSONRPCRequest {
std::string URI;
std::string authUser;
std::string peerAddr;
const std::any &context;

explicit JSONRPCRequest(const std::any &contextIn)
: id(NullUniValue), params(NullUniValue), context(contextIn) {}

//! Initializes request information from another request object and the
//! given context. The implementation should be updated if any members are
//! added or removed above.
JSONRPCRequest(const JSONRPCRequest &other, const std::any &contextIn)
: id(other.id), strMethod(other.strMethod), params(other.params),
mode(other.mode), URI(other.URI), authUser(other.authUser),
peerAddr(other.peerAddr), context(contextIn) {}
std::any context;

void parse(const UniValue &valRequest);
};
Expand Down
11 changes: 6 additions & 5 deletions src/test/rpc_server_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,16 @@ BOOST_AUTO_TEST_CASE(rpc_server_execute_command) {
args.pushKV("arg1", "value1");

// Registered commands execute and return values correctly
std::any context{&m_node};
JSONRPCRequest request(context);
JSONRPCRequest request;
request.context = &m_node;
request.strMethod = commandName;
request.params = args;
UniValue output = rpcServer.ExecuteCommand(config, request);
BOOST_CHECK_EQUAL(output.get_str(), "testing1");

// Not-registered commands throw an exception as expected
JSONRPCRequest badCommandRequest(context);
JSONRPCRequest badCommandRequest;
badCommandRequest.context = &m_node;
badCommandRequest.strMethod = "this-command-does-not-exist";
BOOST_CHECK_EXCEPTION(rpcServer.ExecuteCommand(config, badCommandRequest),
UniValue, isRpcMethodNotFound);
Expand Down Expand Up @@ -88,8 +89,8 @@ BOOST_AUTO_TEST_CASE(rpc_server_execute_command_from_request_context) {
args.pushKV("arg2", "value2");

// Registered commands execute and return values correctly
std::any context{&m_node};
JSONRPCRequest request(context);
JSONRPCRequest request;
request.context = &m_node;
request.strMethod = commandName;
request.params = args;
UniValue output = rpcServer.ExecuteCommand(config, request);
Expand Down
18 changes: 8 additions & 10 deletions src/test/rpc_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@

#include <any>

UniValue CallRPC(const std::string &args, const std::any &context) {
class RPCTestingSetup : public TestingSetup {
public:
UniValue CallRPC(const std::string &args);
};

UniValue RPCTestingSetup::CallRPC(const std::string &args) {
std::vector<std::string> vArgs;
boost::split(vArgs, args, boost::is_any_of(" \t"));
std::string strMethod = vArgs[0];
vArgs.erase(vArgs.begin());
GlobalConfig config;
JSONRPCRequest request(context);
JSONRPCRequest request;
request.context = &m_node;
request.strMethod = strMethod;
request.params = RPCConvertValues(strMethod, vArgs);
if (RPCIsInWarmup(nullptr)) {
Expand All @@ -41,14 +47,6 @@ UniValue CallRPC(const std::string &args, const std::any &context) {
}
}

class RPCTestingSetup : public TestingSetup {
public:
UniValue CallRPC(const std::string &args) {
const std::any context{&m_node};
return ::CallRPC(args, context);
}
};

BOOST_FIXTURE_TEST_SUITE(rpc_tests, RPCTestingSetup)

BOOST_AUTO_TEST_CASE(rpc_rawparams) {
Expand Down
9 changes: 3 additions & 6 deletions src/wallet/test/wallet_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) {
newTip->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1);
key.pushKV("internal", UniValue(true));
keys.push_back(key);
std::any context;
JSONRPCRequest request(context);
JSONRPCRequest request;
request.params.setArray();
request.params.push_back(keys);

Expand Down Expand Up @@ -298,8 +297,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) {
::ChainActive().Height(),
::ChainActive().Tip()->GetBlockHash());
}
std::any context;
JSONRPCRequest request(context);
JSONRPCRequest request;
request.params.setArray();
request.params.push_back(backup_file);
::dumpwallet().HandleRequest(GetConfig(), request);
Expand All @@ -314,8 +312,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) {
LOCK(wallet->cs_wallet);
wallet->SetupLegacyScriptPubKeyMan();

std::any context;
JSONRPCRequest request(context);
JSONRPCRequest request;
request.params.setArray();
request.params.push_back(backup_file);
AddWallet(wallet);
Expand Down

0 comments on commit 5ae5480

Please sign in to comment.