Skip to content

refactor: proper support for composite commands such as 'bls generate' #6051

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 2 commits into from
Jun 10, 2024
Merged

refactor: proper support for composite commands such as 'bls generate' #6051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4bc19f
refactor: proper support for composite commands such as 'bls generate'
knst Jun 5, 2024
9413ecd
fix: disable linter 'check-rpc-mapping.py' for composite commands
knst Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/interfaces/node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ class Node
virtual UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) = 0;

//! List rpc commands.
virtual std::vector<std::string> listRpcCommands() = 0;
virtual std::vector<std::pair<std::string, std::string>> listRpcCommands() = 0;

//! Set RPC timer interface if unset.
virtual void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) = 0;
Expand Down
6 changes: 3 additions & 3 deletions src/node/interfaces.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ class NodeImpl : public Node
req.URI = uri;
return ::tableRPC.execute(req);
}
std::vector<std::string> listRpcCommands() override { return ::tableRPC.listCommands(); }
std::vector<std::pair<std::string, std::string>> listRpcCommands() override { return ::tableRPC.listCommands(); }
void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) override { RPCSetTimerInterfaceIfUnset(iface); }
void rpcUnsetTimerInterface(RPCTimerInterface* iface) override { RPCUnsetTimerInterface(iface); }
bool getUnspentOutput(const COutPoint& output, Coin& coin) override
Expand Down Expand Up @@ -710,14 +710,14 @@ class RpcHandlerImpl : public Handler
throw;
}
};
::tableRPC.appendCommand(m_command.name, &m_command);
::tableRPC.appendCommand(m_command.name, m_command.subname, &m_command);
}

void disconnect() override final
{
if (m_wrapped_command) {
m_wrapped_command = nullptr;
::tableRPC.removeCommand(m_command.name, &m_command);
::tableRPC.removeCommand(m_command.name, m_command.subname, &m_command);
}
}

Expand Down
10 changes: 7 additions & 3 deletions src/qt/rpcconsole.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,11 +705,15 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_

//Setup autocomplete and attach it
QStringList wordList;
std::vector<std::string> commandList = m_node.listRpcCommands();
std::vector<std::pair<std::string, std::string>> commandList = m_node.listRpcCommands();
for (size_t i = 0; i < commandList.size(); ++i)
{
wordList << commandList[i].c_str();
wordList << ("help " + commandList[i]).c_str();
std::string command = commandList[i].first;
if (!commandList[i].second.empty()) {
command = command + " " + commandList[i].second;
}
wordList << command.c_str();
wordList << ("help " + command).c_str();
}

wordList << "help-console";
Expand Down
59 changes: 25 additions & 34 deletions src/rpc/evo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1706,9 +1706,9 @@ static UniValue protx(const JSONRPCRequest& request)
}
}

static void bls_generate_help(const JSONRPCRequest& request)
static RPCHelpMan bls_generate()
{
RPCHelpMan{"bls generate",
return RPCHelpMan{"bls generate",
"\nReturns a BLS secret/public key pair.\n",
{
{"legacy", RPCArg::Type::BOOL, /* default */ "true until the v19 fork is activated, otherwise false", "Use legacy BLS scheme"},
Expand All @@ -1723,12 +1723,10 @@ static void bls_generate_help(const JSONRPCRequest& request)
RPCExamples{
HelpExampleCli("bls generate", "")
},
}.Check(request);
}

static UniValue bls_generate(const JSONRPCRequest& request, const ChainstateManager& chainman)
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
bls_generate_help(request);
const NodeContext& node = EnsureAnyNodeContext(request.context);
const ChainstateManager& chainman = EnsureChainman(node);

CBLSSecretKey sk;
sk.MakeNewKey();
Expand All @@ -1742,11 +1740,13 @@ static UniValue bls_generate(const JSONRPCRequest& request, const ChainstateMana
std::string bls_scheme_str = bls_legacy_scheme ? "legacy" : "basic";
ret.pushKV("scheme", bls_scheme_str);
return ret;
},
};
}

static void bls_fromsecret_help(const JSONRPCRequest& request)
static RPCHelpMan bls_fromsecret()
{
RPCHelpMan{"bls fromsecret",
return RPCHelpMan{"bls fromsecret",
"\nParses a BLS secret key and returns the secret/public key pair.\n",
{
{"secret", RPCArg::Type::STR, RPCArg::Optional::NO, "The BLS secret key"},
Expand All @@ -1762,12 +1762,10 @@ static void bls_fromsecret_help(const JSONRPCRequest& request)
RPCExamples{
HelpExampleCli("bls fromsecret", "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f")
},
}.Check(request);
}

static UniValue bls_fromsecret(const JSONRPCRequest& request, const ChainstateManager& chainman)
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
bls_fromsecret_help(request);
const NodeContext& node = EnsureAnyNodeContext(request.context);
const ChainstateManager& chainman = EnsureChainman(node);

bool bls_legacy_scheme{!DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
if (!request.params[1].isNull()) {
Expand All @@ -1780,11 +1778,13 @@ static UniValue bls_fromsecret(const JSONRPCRequest& request, const ChainstateMa
std::string bls_scheme_str = bls_legacy_scheme ? "legacy" : "basic";
ret.pushKV("scheme", bls_scheme_str);
return ret;
},
};
}

[[ noreturn ]] static void bls_help()
static RPCHelpMan bls_help()
{
RPCHelpMan{"bls",
return RPCHelpMan{"bls",
"Set of commands to execute BLS related actions.\n"
"To get help on individual commands, use \"help bls command\".\n"
"\nAvailable commands:\n"
Expand All @@ -1795,35 +1795,26 @@ static UniValue bls_fromsecret(const JSONRPCRequest& request, const ChainstateMa
},
RPCResults{},
RPCExamples{""},
}.Throw();
}

static UniValue _bls(const JSONRPCRequest& request)
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const JSONRPCRequest new_request{request.strMethod == "bls" ? request.squashed() : request};
const std::string command{new_request.strMethod};

const ChainstateManager& chainman = EnsureAnyChainman(request.context);

if (command == "blsgenerate") {
return bls_generate(new_request, chainman);
} else if (command == "blsfromsecret") {
return bls_fromsecret(new_request, chainman);
} else {
bls_help();
}
throw JSONRPCError(RPC_INVALID_PARAMETER, "Must be a valid command");
},
};
}

void RegisterEvoRPCCommands(CRPCTable &tableRPC)
{
// clang-format off
static const CRPCCommand commands[] =
{ // category name actor (function)
// --------------------- ------------------------ -----------------------
{ "evo", "bls", &_bls, {} },
{ "evo", "bls", &bls_help, {"command"} },
{ "evo", "bls", "generate", &bls_generate, {"legacy"} },
{ "evo", "bls", "fromsecret", &bls_fromsecret, {"secret", "legacy"} },
{ "evo", "protx", &protx, {} },
};
// clang-format on
for (const auto& command : commands) {
tableRPC.appendCommand(command.name, &command);
tableRPC.appendCommand(command.name, command.subname, &command);
}
}
34 changes: 26 additions & 8 deletions src/rpc/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ std::string CRPCTable::help(const std::string& strCommand, const std::string& st
std::vector<std::pair<std::string, const CRPCCommand*> > vCommands;

for (const auto& entry : mapCommands)
vCommands.push_back(make_pair(entry.second.front()->category + entry.first, entry.second.front()));
vCommands.push_back(make_pair(entry.second.front()->category + entry.first.first + entry.first.second, entry.second.front()));
sort(vCommands.begin(), vCommands.end());

JSONRPCRequest jreq = helpreq;
Expand All @@ -105,6 +105,9 @@ std::string CRPCTable::help(const std::string& strCommand, const std::string& st
std::string strMethod = pcmd->name;
if ((strCommand != "" || pcmd->category == "hidden") && strMethod != strCommand)
continue;

if (strSubCommand != pcmd->subname) continue;

jreq.strMethod = strMethod;
try
{
Expand Down Expand Up @@ -294,15 +297,20 @@ CRPCTable::CRPCTable()
}

void CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd)
{
appendCommand(name, "", pcmd);
}

void CRPCTable::appendCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd)
{
CHECK_NONFATAL(!IsRPCRunning()); // Only add commands before rpc is running

mapCommands[name].push_back(pcmd);
mapCommands[std::make_pair(name, subname)].push_back(pcmd);
}

bool CRPCTable::removeCommand(const std::string& name, const CRPCCommand* pcmd)
bool CRPCTable::removeCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd)
{
auto it = mapCommands.find(name);
auto it = mapCommands.find(std::make_pair(name, subname));
if (it != mapCommands.end()) {
auto new_end = std::remove(it->second.begin(), it->second.end(), pcmd);
if (it->second.end() != new_end) {
Expand Down Expand Up @@ -498,12 +506,22 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const
throw JSONRPCError(RPC_IN_WARMUP, rpcWarmupStatus);
}

std::string subcommand;
if (request.params.size() > 0 && request.params[0].isStr()) {
subcommand = request.params[0].get_str();
}

// Find method
auto it = mapCommands.find(request.strMethod);
auto it = mapCommands.find(std::make_pair(request.strMethod, subcommand));
if (it == mapCommands.end() && !subcommand.empty()) {
subcommand = "";
it = mapCommands.find(std::make_pair(request.strMethod, subcommand));
}
if (it != mapCommands.end()) {
UniValue result;
for (const auto& command : it->second) {
if (ExecuteCommand(*command, request, result, &command == &it->second.back(), mapPlatformRestrictions)) {
const JSONRPCRequest new_request{subcommand.empty() ? request : request.squashed() };
if (ExecuteCommand(*command, new_request, result, &command == &it->second.back(), mapPlatformRestrictions)) {
return result;
}
}
Expand Down Expand Up @@ -591,9 +609,9 @@ static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& req
}
}

std::vector<std::string> CRPCTable::listCommands() const
std::vector<std::pair<std::string, std::string>> CRPCTable::listCommands() const
{
std::vector<std::string> commandList;
std::vector<std::pair<std::string, std::string>> commandList;
for (const auto& i : mapCommands) commandList.emplace_back(i.first);
return commandList;
}
Expand Down
34 changes: 28 additions & 6 deletions src/rpc/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ class CRPCCommand
using Actor = std::function<bool(const JSONRPCRequest& request, UniValue& result, bool last_handler)>;

//! Constructor taking Actor callback supporting multiple handlers.
CRPCCommand(std::string category, std::string name, Actor actor, std::vector<std::string> args, intptr_t unique_id)
: category(std::move(category)), name(std::move(name)), actor(std::move(actor)), argNames(std::move(args)),
CRPCCommand(std::string category, std::string name, std::string subname, Actor actor, std::vector<std::string> args, intptr_t unique_id)
: category(std::move(category)), name(std::move(name)), subname(subname), actor(std::move(actor)), argNames(std::move(args)),
unique_id(unique_id)
{
}
Expand All @@ -106,6 +106,7 @@ class CRPCCommand
: CRPCCommand(
category,
fn().m_name,
"",
[fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn().HandleRequest(request); return true; },
fn().GetArgNames(),
intptr_t(fn))
Expand All @@ -114,16 +115,36 @@ class CRPCCommand
CHECK_NONFATAL(fn().GetArgNames() == args_in);
}

//! Simplified constructor taking plain RpcMethodFnType function pointer with sub-command.
CRPCCommand(std::string category, std::string name_in, std::string subname_in, RpcMethodFnType fn, std::vector<std::string> args_in)
: CRPCCommand(
category,
name_in,
subname_in,
[fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn().HandleRequest(request); return true; },
fn().GetArgNames(),
intptr_t(fn))
{
if (subname_in.empty()) {
CHECK_NONFATAL(fn().m_name == name_in);
} else {
CHECK_NONFATAL(fn().m_name == name_in + " " + subname_in);
}

CHECK_NONFATAL(fn().GetArgNames() == args_in);
}

//! Simplified constructor taking plain rpcfn_type function pointer.
CRPCCommand(const char* category, const char* name, rpcfn_type fn, std::initializer_list<const char*> args)
: CRPCCommand(category, name,
: CRPCCommand(category, name, "",
[fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn(request); return true; },
{args.begin(), args.end()}, intptr_t(fn))
{
}

std::string category;
std::string name;
std::string subname;
Actor actor;
std::vector<std::string> argNames;
intptr_t unique_id;
Expand All @@ -135,7 +156,7 @@ class CRPCCommand
class CRPCTable
{
private:
std::map<std::string, std::vector<const CRPCCommand*>> mapCommands;
std::map<std::pair<std::string, std::string>, std::vector<const CRPCCommand*>> mapCommands;
std::multimap<std::string, std::vector<UniValue>> mapPlatformRestrictions;
public:
CRPCTable();
Expand All @@ -155,7 +176,7 @@ class CRPCTable
* Returns a list of registered commands
* @returns List of registered commands.
*/
std::vector<std::string> listCommands() const;
std::vector<std::pair<std::string, std::string>> listCommands() const;

/**
* Appends a CRPCCommand to the dispatch table.
Expand All @@ -170,7 +191,8 @@ class CRPCTable
* register different names, types, and numbers of parameters.
*/
void appendCommand(const std::string& name, const CRPCCommand* pcmd);
bool removeCommand(const std::string& name, const CRPCCommand* pcmd);
void appendCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd);
bool removeCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd);
};

bool IsDeprecatedRPCEnabled(const std::string& method);
Expand Down
2 changes: 1 addition & 1 deletion src/test/rpc_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector<st
{
UniValue transformed_params;
CRPCTable table;
CRPCCommand command{"category", "method", [&](const JSONRPCRequest& request, UniValue&, bool) -> bool { transformed_params = request.params; return true; }, arg_names, /*unique_id=*/0};
CRPCCommand command{"category", "method", "subcommand", [&](const JSONRPCRequest& request, UniValue&, bool) -> bool { transformed_params = request.params; return true; }, arg_names, /*unique_id=*/0};
table.appendCommand("method", &command);
CoreContext context{m_node};
JSONRPCRequest request(context);
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/interfaces.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ class WalletLoaderImpl : public WalletLoader
void registerRpcs() override
{
for (const CRPCCommand& command : GetWalletRPCCommands()) {
m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) {
m_rpc_commands.emplace_back(command.category, command.name, command.subname, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) {
return command.actor({request, m_context}, result, last_handler);
}, command.argNames, command.unique_id);
m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back()));
Expand Down
6 changes: 6 additions & 0 deletions test/lint/check-rpc-mappings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ def process_commands(fname):
in_rpcs = False
elif '{' in line and '"' in line:
m = re.search(r'{ *("[^"]*"), *("[^"]*"), *&([^,]*), *{([^}]*)} *},', line)
# that's a quick fix for composite command
# no proper implementation is needed so far as this linter would be removed soon with bitcoin#20012
if not m:
m = re.search(r'{ *("[^"]*"), *("[^"]*"), *("[^"]*"), *&([^,]*), *{([^}]*)} *},', line)
if m:
continue
assert m, 'No match to table expression: %s' % line
name = parse_string(m.group(2))
args_str = m.group(4).strip()
Expand Down