Merge bitcoin/bitcoin#26039: refactor: Run type check against RPCArgs (1/2)

fa9f6d7bcd rpc: Run type check against RPCArgs (MarcoFalke)
faf96721a6 test: Fix wrong types passed to RPCs (MarcoFalke)

Pull request description:

  It seems brittle to require `RPCTypeCheck` being called inside the code logic. Without compile-time enforcement this will lead to places where it is forgotten and thus to inconsistencies and bugs. Fix this by removing the calls to `RPCTypeCheck` and doing the check internally.

  The changes should be reviewed as refactoring changes. However, if they change behavior, it will be a bugfix. For example the changes here happen to also detect/fix bugs like the one fixed in commit 3b5fb6e77a.

ACKs for top commit:
  ajtowns:
    ACK fa9f6d7bcd

Tree-SHA512: fb2c0981fe6e24da3ca7dbc06898730779ea4e02ea485458505a281cf421015e44dad0221a04023fc547ea2c660d94657909843fc85d92b847611ec097532439
This commit is contained in:
fanquake
2023-01-17 09:33:14 +00:00
14 changed files with 85 additions and 155 deletions

View File

@@ -53,7 +53,6 @@ static RPCHelpMan setmocktime()
// ensure all call sites of GetTime() are accessing this safely.
LOCK(cs_main);
RPCTypeCheck(request.params, {UniValue::VNUM});
const int64_t time{request.params[0].getInt<int64_t>()};
if (time < 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Mocktime cannot be negative: %s.", time));
@@ -107,8 +106,6 @@ static RPCHelpMan mockscheduler()
throw std::runtime_error("mockscheduler is for regression testing (-regtest mode) only");
}
// check params are valid values
RPCTypeCheck(request.params, {UniValue::VNUM});
int64_t delta_seconds = request.params[0].getInt<int64_t>();
if (delta_seconds <= 0 || delta_seconds > 3600) {
throw std::runtime_error("delta_time must be between 1 and 3600 seconds (1 hr)");
@@ -296,18 +293,18 @@ static RPCHelpMan echo(const std::string& name)
"\nIt will return an internal bug report when arg9='trigger_internal_bug' is passed.\n"
"\nThe difference between echo and echojson is that echojson has argument conversion enabled in the client-side table in "
"bitcoin-cli and the GUI. There is no server-side difference.",
{
{"arg0", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg1", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg4", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg5", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg6", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg7", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
{"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
},
{
{"arg0", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg1", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg4", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg5", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg6", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg7", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
{"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
},
RPCResult{RPCResult::Type::ANY, "", "Returns whatever was passed in"},
RPCExamples{""},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue