Skip to content

Commit fa3169b

Browse files
author
MarcoFalke
committed
rpc: Remove index-based Arg accessor
1 parent 42d5a1f commit fa3169b

File tree

7 files changed

+28
-58
lines changed

7 files changed

+28
-58
lines changed

‎src/rpc/mempool.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ static RPCHelpMan sendrawtransaction()
8282

8383
CTransactionRef tx(MakeTransactionRef(std::move(mtx)));
8484

85-
const CFeeRate max_raw_tx_fee_rate{ParseFeeRate(self.Arg<UniValue>(1))};
85+
const CFeeRate max_raw_tx_fee_rate{ParseFeeRate(self.Arg<UniValue>("maxfeerate"))};
8686

8787
int64_t virtual_size = GetVirtualTransactionSize(*tx);
8888
CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size);
@@ -162,7 +162,7 @@ static RPCHelpMan testmempoolaccept()
162162
"Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
163163
}
164164

165-
const CFeeRate max_raw_tx_fee_rate{ParseFeeRate(self.Arg<UniValue>(1))};
165+
const CFeeRate max_raw_tx_fee_rate{ParseFeeRate(self.Arg<UniValue>("maxfeerate"))};
166166

167167
std::vector<CTransactionRef> txns;
168168
txns.reserve(raw_transactions.size());
@@ -873,7 +873,7 @@ static RPCHelpMan submitpackage()
873873
}
874874

875875
// Fee check needs to be run with chainstate and package context
876-
const CFeeRate max_raw_tx_fee_rate = ParseFeeRate(self.Arg<UniValue>(1));
876+
const CFeeRate max_raw_tx_fee_rate{ParseFeeRate(self.Arg<UniValue>("maxfeerate"))};
877877
std::optional<CFeeRate> client_maxfeerate{max_raw_tx_fee_rate};
878878
// 0-value is special; it's mapped to no sanity check
879879
if (max_raw_tx_fee_rate == CFeeRate(0)) {

‎src/rpc/mining.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ static RPCHelpMan prioritisetransaction()
485485
LOCK(cs_main);
486486

487487
uint256 hash(ParseHashV(request.params[0], "txid"));
488-
const auto dummy{self.MaybeArg<double>(1)};
488+
const auto dummy{self.MaybeArg<double>("dummy")};
489489
CAmount nAmount = request.params[2].getInt<int64_t>();
490490

491491
if (dummy && *dummy != 0) {

‎src/rpc/net.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ static RPCHelpMan addconnection()
401401
} else {
402402
throw JSONRPCError(RPC_INVALID_PARAMETER, self.ToString());
403403
}
404-
bool use_v2transport = self.Arg<bool>(2);
404+
bool use_v2transport{self.Arg<bool>("v2transport")};
405405

406406
NodeContext& node = EnsureAnyNodeContext(request.context);
407407
CConnman& connman = EnsureConnman(node);

‎src/rpc/util.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ static const UniValue* DetailMaybeArg(CheckFn* check, const std::vector<RPCArg>&
677677

678678
static void CheckRequiredOrDefault(const RPCArg& param)
679679
{
680-
// Must use `Arg<Type>(i)` to get the argument or its default value.
680+
// Must use `Arg<Type>(key)` to get the argument or its default value.
681681
const bool required{
682682
std::holds_alternative<RPCArg::Optional>(param.m_fallback) && RPCArg::Optional::NO == std::get<RPCArg::Optional>(param.m_fallback),
683683
};

‎src/rpc/util.h‎

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -414,19 +414,16 @@ class RPCHelpMan
414414
* argument isNull() and parses (from JSON) and returns the user-passed argument,
415415
* or the default value derived from the RPCArg documentation.
416416
*
417-
* There are two overloads of this function:
418-
* - Use Arg<Type>(size_t i) to get the argument (or the default value) by index.
419-
* - Use Arg<Type>(const std::string& key) to get the argument (or the default value) by key.
417+
* The instantiation of this helper for type R must match the corresponding RPCArg::Type.
420418
*
421-
* The Type passed to this helper must match the corresponding RPCArg::Type.
422-
*
423-
* @return The value of the RPC argument (or the default value) cast to type Type.
419+
* @return The value of the RPC argument (or the default value) cast to type R.
424420
*
425421
* @see MaybeArg for handling optional arguments without default values.
426422
*/
427423
template <typename R>
428-
auto Arg(size_t i) const
424+
auto Arg(std::string_view key) const
429425
{
426+
auto i{GetParamIndex(key)};
430427
// Return argument (required or with default value).
431428
if constexpr (std::is_integral_v<R> || std::is_floating_point_v<R>) {
432429
// Return numbers by value.
@@ -436,11 +433,6 @@ class RPCHelpMan
436433
return ArgValue<const R&>(i);
437434
}
438435
}
439-
template<typename R>
440-
auto Arg(std::string_view key) const
441-
{
442-
return Arg<R>(GetParamIndex(key));
443-
}
444436
/**
445437
* @brief Helper to get an optional request argument.
446438
*
@@ -452,21 +444,18 @@ class RPCHelpMan
452444
* argument isNull() and parses (from JSON) and returns the user-passed argument,
453445
* or a falsy value if no argument was passed.
454446
*
455-
* There are two overloads of this function:
456-
* - Use MaybeArg<Type>(size_t i) to get the optional argument by index.
457-
* - Use MaybeArg<Type>(const std::string& key) to get the optional argument by key.
447+
* The instantiation of this helper for type R must match the corresponding RPCArg::Type.
458448
*
459-
* The Type passed to this helper must match the corresponding RPCArg::Type.
449+
* @return For integral and floating-point types, a std::optional<R> is returned.
450+
* For other types, a R* pointer to the argument is returned. If the
451+
* argument is not provided, std::nullopt or a null pointer is returned.
460452
*
461-
* @return For integral and floating-point types, a std::optional<Type> is returned.
462-
* For other types, a Type* pointer to the argument is returned. If the
463-
* argument is not provided, std::nullopt or a null pointer is returned.
464-
*
465453
* @see Arg for handling arguments that are required or have a default value.
466454
*/
467455
template <typename R>
468-
auto MaybeArg(size_t i) const
456+
auto MaybeArg(std::string_view key) const
469457
{
458+
auto i{GetParamIndex(key)};
470459
// Return optional argument (without default).
471460
if constexpr (std::is_integral_v<R> || std::is_floating_point_v<R>) {
472461
// Return numbers by value, wrapped in optional.
@@ -476,11 +465,6 @@ class RPCHelpMan
476465
return ArgValue<const R*>(i);
477466
}
478467
}
479-
template<typename R>
480-
auto MaybeArg(std::string_view key) const
481-
{
482-
return MaybeArg<R>(GetParamIndex(key));
483-
}
484468
std::string ToString() const;
485469
/** Return the named args that need to be converted from string to another JSON type */
486470
UniValue GetArgMap() const;

‎src/test/rpc_tests.cpp‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -614,40 +614,26 @@ BOOST_AUTO_TEST_CASE(rpc_arg_helper)
614614

615615
//! Check that `self.Arg` returns the same value as the `request.params` accessors
616616
RPCHelpMan::RPCMethodImpl check_positional = [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
617-
BOOST_CHECK_EQUAL(self.Arg<int>(0), request.params[0].getInt<int>());
618-
BOOST_CHECK_EQUAL(self.Arg<std::string>(1), request.params[1].get_str());
619-
BOOST_CHECK_EQUAL(self.Arg<uint64_t>(2), request.params[2].isNull() ? DEFAULT_UINT64_T : request.params[2].getInt<uint64_t>());
620-
BOOST_CHECK_EQUAL(self.Arg<std::string>(3), request.params[3].isNull() ? DEFAULT_STRING : request.params[3].get_str());
621-
BOOST_CHECK_EQUAL(self.Arg<bool>(4), request.params[4].isNull() ? DEFAULT_BOOL : request.params[4].get_bool());
617+
BOOST_CHECK_EQUAL(self.Arg<int>("req_int"), request.params[0].getInt<int>());
618+
BOOST_CHECK_EQUAL(self.Arg<std::string>("req_str"), request.params[1].get_str());
619+
BOOST_CHECK_EQUAL(self.Arg<uint64_t>("def_uint64_t"), request.params[2].isNull() ? DEFAULT_UINT64_T : request.params[2].getInt<uint64_t>());
620+
BOOST_CHECK_EQUAL(self.Arg<std::string>("def_string"), request.params[3].isNull() ? DEFAULT_STRING : request.params[3].get_str());
621+
BOOST_CHECK_EQUAL(self.Arg<bool>("def_bool"), request.params[4].isNull() ? DEFAULT_BOOL : request.params[4].get_bool());
622622
if (!request.params[5].isNull()) {
623-
BOOST_CHECK_EQUAL(self.MaybeArg<double>(5).value(), request.params[5].get_real());
623+
BOOST_CHECK_EQUAL(self.MaybeArg<double>("opt_double").value(), request.params[5].get_real());
624624
} else {
625-
BOOST_CHECK(!self.MaybeArg<double>(5));
625+
BOOST_CHECK(!self.MaybeArg<double>("opt_double"));
626626
}
627627
if (!request.params[6].isNull()) {
628-
BOOST_CHECK(self.MaybeArg<std::string>(6));
629-
BOOST_CHECK_EQUAL(*self.MaybeArg<std::string>(6), request.params[6].get_str());
628+
BOOST_CHECK(self.MaybeArg<std::string>("opt_string"));
629+
BOOST_CHECK_EQUAL(*self.MaybeArg<std::string>("opt_string"), request.params[6].get_str());
630630
} else {
631-
BOOST_CHECK(!self.MaybeArg<std::string>(6));
631+
BOOST_CHECK(!self.MaybeArg<std::string>("opt_string"));
632632
}
633633
return UniValue{};
634634
};
635635
CheckRpc(params, UniValue{JSON(R"([5, "hello", null, null, null, null, null])")}, check_positional);
636636
CheckRpc(params, UniValue{JSON(R"([5, "hello", 4, "test", true, 1.23, "world"])")}, check_positional);
637-
638-
//! Check that `self.Arg` returns the same value when using index and key
639-
RPCHelpMan::RPCMethodImpl check_named = [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
640-
BOOST_CHECK_EQUAL(self.Arg<int>(0), self.Arg<int>("req_int"));
641-
BOOST_CHECK_EQUAL(self.Arg<std::string>(1), self.Arg<std::string>("req_str"));
642-
BOOST_CHECK_EQUAL(self.Arg<uint64_t>(2), self.Arg<uint64_t>("def_uint64_t"));
643-
BOOST_CHECK_EQUAL(self.Arg<std::string>(3), self.Arg<std::string>("def_string"));
644-
BOOST_CHECK_EQUAL(self.Arg<bool>(4), self.Arg<bool>("def_bool"));
645-
BOOST_CHECK(self.MaybeArg<double>(5) == self.MaybeArg<double>("opt_double"));
646-
BOOST_CHECK(self.MaybeArg<std::string>(6) == self.MaybeArg<std::string>("opt_string"));
647-
return UniValue{};
648-
};
649-
CheckRpc(params, UniValue{JSON(R"([5, "hello", null, null, null, null, null])")}, check_named);
650-
CheckRpc(params, UniValue{JSON(R"([5, "hello", 4, "test", true, 1.23, "world"])")}, check_named);
651637
}
652638

653639
BOOST_AUTO_TEST_SUITE_END()

‎src/wallet/rpc/wallet.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ static RPCHelpMan createwallet()
395395
if (!request.params[4].isNull() && request.params[4].get_bool()) {
396396
flags |= WALLET_FLAG_AVOID_REUSE;
397397
}
398-
if (self.Arg<bool>(5)) {
398+
if (self.Arg<bool>("descriptors")) {
399399
#ifndef USE_SQLITE
400400
throw JSONRPCError(RPC_WALLET_ERROR, "Compiled without sqlite support (required for descriptor wallets)");
401401
#endif
@@ -489,7 +489,7 @@ static RPCHelpMan unloadwallet()
489489
// Release the "main" shared pointer and prevent further notifications.
490490
// Note that any attempt to load the same wallet would fail until the wallet
491491
// is destroyed (see CheckUniqueFileid).
492-
std::optional<bool> load_on_start{self.MaybeArg<bool>(1)};
492+
std::optional<bool> load_on_start{self.MaybeArg<bool>("load_on_startup")};
493493
if (!RemoveWallet(context, wallet, load_on_start, warnings)) {
494494
throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded");
495495
}

0 commit comments

Comments
 (0)