rpc: refactor: use string_view in Arg/MaybeArg

Modernizes interface by not forcing users to deal with raw pointers,
without adding copying overhead. Generalizes the logic of whether
we return by value or by optional/pointer.

In cases where functions take a `const std::string&` and it would
be too much work to update them, a string copy is made (which was
already happening anyway).
This commit is contained in:
stickies-v 2025-07-14 16:53:46 +01:00
parent 75353a0163
commit b3bf18f0ba
No known key found for this signature in database
GPG Key ID: 5CB1CE6E5E66A757
18 changed files with 51 additions and 46 deletions

View File

@ -12,6 +12,7 @@
#include <cstdint>
#include <memory>
#include <string>
#include <string_view>
#include <utility>
#include <vector>
@ -19,7 +20,7 @@ static void ExpandDescriptor(benchmark::Bench& bench)
{
ECC_Context ecc_context{};
const auto desc_str = "sh(wsh(multi(16,03669b8afcec803a0d323e9a17f3ea8e68e8abe5a278020a929adbec52421adbd0,0260b2003c386519fc9eadf2b5cf124dd8eea4c4e68d5e154050a9346ea98ce600,0362a74e399c39ed5593852a30147f2959b56bb827dfa3e60e464b02ccf87dc5e8,0261345b53de74a4d721ef877c255429961b7e43714171ac06168d7e08c542a8b8,02da72e8b46901a65d4374fe6315538d8f368557dda3a1dcf9ea903f3afe7314c8,0318c82dd0b53fd3a932d16e0ba9e278fcc937c582d5781be626ff16e201f72286,0297ccef1ef99f9d73dec9ad37476ddb232f1238aff877af19e72ba04493361009,02e502cfd5c3f972fe9a3e2a18827820638f96b6f347e54d63deb839011fd5765d,03e687710f0e3ebe81c1037074da939d409c0025f17eb86adb9427d28f0f7ae0e9,02c04d3a5274952acdbc76987f3184b346a483d43be40874624b29e3692c1df5af,02ed06e0f418b5b43a7ec01d1d7d27290fa15f75771cb69b642a51471c29c84acd,036d46073cbb9ffee90473f3da429abc8de7f8751199da44485682a989a4bebb24,02f5d1ff7c9029a80a4e36b9a5497027ef7f3e73384a4a94fbfe7c4e9164eec8bc,02e41deffd1b7cce11cde209a781adcffdabd1b91c0ba0375857a2bfd9302419f3,02d76625f7956a7fc505ab02556c23ee72d832f1bac391bcd2d3abce5710a13d06,0399eb0a5487515802dc14544cf10b3666623762fbed2ec38a3975716e2c29c232)))";
constexpr std::string_view desc_str{"sh(wsh(multi(16,03669b8afcec803a0d323e9a17f3ea8e68e8abe5a278020a929adbec52421adbd0,0260b2003c386519fc9eadf2b5cf124dd8eea4c4e68d5e154050a9346ea98ce600,0362a74e399c39ed5593852a30147f2959b56bb827dfa3e60e464b02ccf87dc5e8,0261345b53de74a4d721ef877c255429961b7e43714171ac06168d7e08c542a8b8,02da72e8b46901a65d4374fe6315538d8f368557dda3a1dcf9ea903f3afe7314c8,0318c82dd0b53fd3a932d16e0ba9e278fcc937c582d5781be626ff16e201f72286,0297ccef1ef99f9d73dec9ad37476ddb232f1238aff877af19e72ba04493361009,02e502cfd5c3f972fe9a3e2a18827820638f96b6f347e54d63deb839011fd5765d,03e687710f0e3ebe81c1037074da939d409c0025f17eb86adb9427d28f0f7ae0e9,02c04d3a5274952acdbc76987f3184b346a483d43be40874624b29e3692c1df5af,02ed06e0f418b5b43a7ec01d1d7d27290fa15f75771cb69b642a51471c29c84acd,036d46073cbb9ffee90473f3da429abc8de7f8751199da44485682a989a4bebb24,02f5d1ff7c9029a80a4e36b9a5497027ef7f3e73384a4a94fbfe7c4e9164eec8bc,02e41deffd1b7cce11cde209a781adcffdabd1b91c0ba0375857a2bfd9302419f3,02d76625f7956a7fc505ab02556c23ee72d832f1bac391bcd2d3abce5710a13d06,0399eb0a5487515802dc14544cf10b3666623762fbed2ec38a3975716e2c29c232)))"};
const std::pair<int64_t, int64_t> range = {0, 1000};
FlatSigningProvider provider;
std::string error;

View File

@ -39,11 +39,12 @@
#include <algorithm>
#include <array>
#include <cstring>
#include <cmath>
#include <cstdint>
#include <cstring>
#include <functional>
#include <optional>
#include <string_view>
#include <unordered_map>
TRACEPOINT_SEMAPHORE(net, closed_connection);
@ -3562,11 +3563,11 @@ bool CConnman::AddNode(const AddedNodeParams& add)
return true;
}
bool CConnman::RemoveAddedNode(const std::string& strNode)
bool CConnman::RemoveAddedNode(std::string_view node)
{
LOCK(m_added_nodes_mutex);
for (auto it = m_added_node_params.begin(); it != m_added_node_params.end(); ++it) {
if (strNode == it->m_added_node) {
if (node == it->m_added_node) {
m_added_node_params.erase(it);
return true;
}

View File

@ -43,6 +43,7 @@
#include <memory>
#include <optional>
#include <queue>
#include <string_view>
#include <thread>
#include <unordered_set>
#include <vector>
@ -1222,7 +1223,7 @@ public:
int GetExtraBlockRelayCount() const;
bool AddNode(const AddedNodeParams& add) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
bool RemoveAddedNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
bool RemoveAddedNode(std::string_view node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
bool AddedNodesContain(const CAddress& addr) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
std::vector<AddedNodeInfo> GetAddedNodeInfo(bool include_connected) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);

View File

@ -60,6 +60,7 @@
#include <memory>
#include <mutex>
#include <optional>
#include <string_view>
#include <vector>
using kernel::CCoinsStats;
@ -2319,7 +2320,7 @@ static RPCHelpMan scantxoutset()
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
UniValue result(UniValue::VOBJ);
const auto action{self.Arg<std::string>("action")};
const auto action{self.Arg<std::string_view>("action")};
if (action == "status") {
CoinsViewScanReserver reserver;
if (reserver.reserve()) {
@ -3055,7 +3056,7 @@ static RPCHelpMan dumptxoutset()
NodeContext& node = EnsureAnyNodeContext(request.context);
const CBlockIndex* tip{WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Tip())};
const CBlockIndex* target_index{nullptr};
const std::string snapshot_type{self.Arg<std::string>("type")};
const auto snapshot_type{self.Arg<std::string_view>("type")};
const UniValue options{request.params[2].isNull() ? UniValue::VOBJ : request.params[2]};
if (options.exists("rollback")) {
if (!snapshot_type.empty() && snapshot_type != "rollback") {
@ -3346,7 +3347,7 @@ static RPCHelpMan loadtxoutset()
{
NodeContext& node = EnsureAnyNodeContext(request.context);
ChainstateManager& chainman = EnsureChainman(node);
const fs::path path{AbsPathForConfigVal(EnsureArgsman(node), fs::u8path(self.Arg<std::string>("path")))};
const fs::path path{AbsPathForConfigVal(EnsureArgsman(node), fs::u8path(self.Arg<std::string_view>("path")))};
FILE* file{fsbridge::fopen(path, "rb")};
AutoFile afile{file};

View File

@ -181,7 +181,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const
return blockHashes;
}
static bool getScriptFromDescriptor(const std::string& descriptor, CScript& script, std::string& error)
static bool getScriptFromDescriptor(std::string_view descriptor, CScript& script, std::string& error)
{
FlatSigningProvider key_provider;
const auto descs = Parse(descriptor, key_provider, error, /* require_checksum = */ false);
@ -241,7 +241,7 @@ static RPCHelpMan generatetodescriptor()
CScript coinbase_output_script;
std::string error;
if (!getScriptFromDescriptor(self.Arg<std::string>("descriptor"), coinbase_output_script, error)) {
if (!getScriptFromDescriptor(self.Arg<std::string_view>("descriptor"), coinbase_output_script, error)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error);
}

View File

@ -333,7 +333,7 @@ static RPCHelpMan addnode()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const auto command{self.Arg<std::string>("command")};
const auto command{self.Arg<std::string_view>("command")};
if (command != "onetry" && command != "add" && command != "remove") {
throw std::runtime_error(
self.ToString());
@ -342,7 +342,7 @@ static RPCHelpMan addnode()
NodeContext& node = EnsureAnyNodeContext(request.context);
CConnman& connman = EnsureConnman(node);
const auto node_arg{self.Arg<std::string>("node")};
const auto node_arg{self.Arg<std::string_view>("node")};
bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2;
bool use_v2transport = self.MaybeArg<bool>("v2transport").value_or(node_v2transport);
@ -353,13 +353,13 @@ static RPCHelpMan addnode()
if (command == "onetry")
{
CAddress addr;
connman.OpenNetworkConnection(addr, /*fCountFailure=*/false, /*grant_outbound=*/{}, node_arg.c_str(), ConnectionType::MANUAL, use_v2transport);
connman.OpenNetworkConnection(addr, /*fCountFailure=*/false, /*grant_outbound=*/{}, std::string{node_arg}.c_str(), ConnectionType::MANUAL, use_v2transport);
return UniValue::VNULL;
}
if (command == "add")
{
if (!connman.AddNode({node_arg, use_v2transport})) {
if (!connman.AddNode({std::string{node_arg}, use_v2transport})) {
throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: Node already added");
}
}

View File

@ -38,11 +38,9 @@ static RPCHelpMan verifymessage()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
std::string strAddress = self.Arg<std::string>("address");
std::string strSign = self.Arg<std::string>("signature");
std::string strMessage = self.Arg<std::string>("message");
switch (MessageVerify(strAddress, strSign, strMessage)) {
switch (MessageVerify(std::string{self.Arg<std::string_view>("address")},
std::string{self.Arg<std::string_view>("signature")},
std::string{self.Arg<std::string_view>("message")})) {
case MessageVerificationResult::ERR_INVALID_ADDRESS:
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
case MessageVerificationResult::ERR_ADDRESS_NO_KEY:

View File

@ -724,7 +724,7 @@ static void CheckRequiredOrDefault(const RPCArg& param)
TMPL_INST(nullptr, const UniValue*, maybe_arg;);
TMPL_INST(nullptr, std::optional<double>, maybe_arg ? std::optional{maybe_arg->get_real()} : std::nullopt;);
TMPL_INST(nullptr, std::optional<bool>, maybe_arg ? std::optional{maybe_arg->get_bool()} : std::nullopt;);
TMPL_INST(nullptr, const std::string*, maybe_arg ? &maybe_arg->get_str() : nullptr;);
TMPL_INST(nullptr, std::optional<std::string_view>, maybe_arg ? std::optional<std::string_view>{maybe_arg->get_str()} : std::nullopt;);
// Required arg or optional arg with default value.
TMPL_INST(CheckRequiredOrDefault, const UniValue&, *CHECK_NONFATAL(maybe_arg););
@ -732,7 +732,7 @@ TMPL_INST(CheckRequiredOrDefault, bool, CHECK_NONFATAL(maybe_arg)->get_bool(););
TMPL_INST(CheckRequiredOrDefault, int, CHECK_NONFATAL(maybe_arg)->getInt<int>(););
TMPL_INST(CheckRequiredOrDefault, uint64_t, CHECK_NONFATAL(maybe_arg)->getInt<uint64_t>(););
TMPL_INST(CheckRequiredOrDefault, uint32_t, CHECK_NONFATAL(maybe_arg)->getInt<uint32_t>(););
TMPL_INST(CheckRequiredOrDefault, const std::string&, CHECK_NONFATAL(maybe_arg)->get_str(););
TMPL_INST(CheckRequiredOrDefault, std::string_view, CHECK_NONFATAL(maybe_arg)->get_str(););
bool RPCHelpMan::IsValidNumArgs(size_t num_args) const
{

View File

@ -445,8 +445,8 @@ public:
{
auto i{GetParamIndex(key)};
// Return argument (required or with default value).
if constexpr (std::is_integral_v<R> || std::is_floating_point_v<R>) {
// Return numbers by value.
if constexpr (std::is_trivially_copyable_v<R>) {
// Return trivially copyable types by value.
return ArgValue<R>(i);
} else {
// Return everything else by reference.
@ -466,7 +466,7 @@ public:
*
* The instantiation of this helper for type R must match the corresponding RPCArg::Type.
*
* @return For integral and floating-point types, a std::optional<R> is returned.
* @return For trivially copyable types, a std::optional<R> is returned.
* For other types, a R* pointer to the argument is returned. If the
* argument is not provided, std::nullopt or a null pointer is returned.
*
@ -477,8 +477,8 @@ public:
{
auto i{GetParamIndex(key)};
// Return optional argument (without default).
if constexpr (std::is_integral_v<R> || std::is_floating_point_v<R>) {
// Return numbers by value, wrapped in optional.
if constexpr (std::is_trivially_copyable_v<R>) {
// Return trivially copyable types by value, wrapped in optional.
return ArgValue<std::optional<R>>(i);
} else {
// Return other types by pointer.

View File

@ -2735,13 +2735,12 @@ bool CheckChecksum(std::span<const char>& sp, bool require_checksum, std::string
return true;
}
std::vector<std::unique_ptr<Descriptor>> Parse(const std::string& descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum)
std::vector<std::unique_ptr<Descriptor>> Parse(std::span<const char> descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum)
{
std::span<const char> sp{descriptor};
if (!CheckChecksum(sp, require_checksum, error)) return {};
if (!CheckChecksum(descriptor, require_checksum, error)) return {};
uint32_t key_exp_index = 0;
auto ret = ParseScript(key_exp_index, sp, ParseScriptContext::TOP, out, error);
if (sp.size() == 0 && !ret.empty()) {
auto ret = ParseScript(key_exp_index, descriptor, ParseScriptContext::TOP, out, error);
if (descriptor.empty() && !ret.empty()) {
std::vector<std::unique_ptr<Descriptor>> descs;
descs.reserve(ret.size());
for (auto& r : ret) {

View File

@ -175,7 +175,7 @@ struct Descriptor {
* If a parse error occurs, or the checksum is missing/invalid, or anything
* else is wrong, an empty vector is returned.
*/
std::vector<std::unique_ptr<Descriptor>> Parse(const std::string& descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum = false);
std::vector<std::unique_ptr<Descriptor>> Parse(std::span<const char> descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum = false);
/** Get the checksum for a `descriptor`.
*

View File

@ -14,6 +14,7 @@
#include <util/time.h>
#include <any>
#include <string_view>
#include <boost/test/unit_test.hpp>
@ -618,9 +619,9 @@ BOOST_AUTO_TEST_CASE(rpc_arg_helper)
//! Check that `self.Arg` returns the same value as the `request.params` accessors
RPCHelpMan::RPCMethodImpl check_positional = [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
BOOST_CHECK_EQUAL(self.Arg<int>("req_int"), request.params[0].getInt<int>());
BOOST_CHECK_EQUAL(self.Arg<std::string>("req_str"), request.params[1].get_str());
BOOST_CHECK_EQUAL(self.Arg<std::string_view>("req_str"), request.params[1].get_str());
BOOST_CHECK_EQUAL(self.Arg<uint64_t>("def_uint64_t"), request.params[2].isNull() ? DEFAULT_UINT64_T : request.params[2].getInt<uint64_t>());
BOOST_CHECK_EQUAL(self.Arg<std::string>("def_string"), request.params[3].isNull() ? DEFAULT_STRING : request.params[3].get_str());
BOOST_CHECK_EQUAL(self.Arg<std::string_view>("def_string"), request.params[3].isNull() ? DEFAULT_STRING : request.params[3].get_str());
BOOST_CHECK_EQUAL(self.Arg<bool>("def_bool"), request.params[4].isNull() ? DEFAULT_BOOL : request.params[4].get_bool());
if (!request.params[5].isNull()) {
BOOST_CHECK_EQUAL(self.MaybeArg<double>("opt_double").value(), request.params[5].get_real());
@ -628,10 +629,9 @@ BOOST_AUTO_TEST_CASE(rpc_arg_helper)
BOOST_CHECK(!self.MaybeArg<double>("opt_double"));
}
if (!request.params[6].isNull()) {
BOOST_CHECK(self.MaybeArg<std::string>("opt_string"));
BOOST_CHECK_EQUAL(*self.MaybeArg<std::string>("opt_string"), request.params[6].get_str());
BOOST_CHECK_EQUAL(self.MaybeArg<std::string_view>("opt_string"), request.params[6].get_str());
} else {
BOOST_CHECK(!self.MaybeArg<std::string>("opt_string"));
BOOST_CHECK(!self.MaybeArg<std::string_view>("opt_string"));
}
return UniValue{};
};

View File

@ -14,6 +14,7 @@
#include <ios>
#include <ostream>
#include <string>
#include <string_view>
#include <system_error>
#include <type_traits>
#include <utility>
@ -72,7 +73,7 @@ public:
path filename() const { return std::filesystem::path::filename(); }
};
static inline path u8path(const std::string& utf8_str)
static inline path u8path(std::string_view utf8_str)
{
return std::filesystem::path(std::u8string{utf8_str.begin(), utf8_str.end()});
}

View File

@ -196,8 +196,7 @@ RPCHelpMan getbalance()
LOCK(pwallet->cs_wallet);
const auto dummy_value{self.MaybeArg<std::string>("dummy")};
if (dummy_value && *dummy_value != "*") {
if (self.MaybeArg<std::string_view>("dummy").value_or("*") != "*") {
throw JSONRPCError(RPC_METHOD_DEPRECATED, "dummy first argument must be excluded or set to \"*\".");
}

View File

@ -11,6 +11,7 @@
#include <wallet/context.h>
#include <wallet/wallet.h>
#include <optional>
#include <string_view>
#include <univalue.h>
@ -29,7 +30,7 @@ bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param) {
return avoid_reuse;
}
std::string EnsureUniqueWalletName(const JSONRPCRequest& request, const std::string* wallet_name)
std::string EnsureUniqueWalletName(const JSONRPCRequest& request, std::optional<std::string_view> wallet_name)
{
std::string endpoint_wallet;
if (GetWalletNameFromJSONRPCRequest(request, endpoint_wallet)) {
@ -47,7 +48,7 @@ std::string EnsureUniqueWalletName(const JSONRPCRequest& request, const std::str
"Either the RPC endpoint wallet or the wallet name parameter must be provided");
}
return *wallet_name;
return std::string{*wallet_name};
}
bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name)

View File

@ -11,7 +11,9 @@
#include <any>
#include <memory>
#include <optional>
#include <string>
#include <string_view>
#include <vector>
class JSONRPCRequest;
@ -42,7 +44,7 @@ bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string&
* Ensures that a wallet name is specified across the endpoint and wallet_name.
* Throws `RPC_INVALID_PARAMETER` if none or different wallet names are specified.
*/
std::string EnsureUniqueWalletName(const JSONRPCRequest& request, const std::string* wallet_name);
std::string EnsureUniqueWalletName(const JSONRPCRequest& request, std::optional<std::string_view> wallet_name);
void EnsureWalletIsUnlocked(const CWallet&);
WalletContext& EnsureWalletContext(const std::any& context);

View File

@ -19,6 +19,7 @@
#include <wallet/walletutil.h>
#include <optional>
#include <string_view>
namespace wallet {
@ -456,7 +457,7 @@ static RPCHelpMan unloadwallet()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const std::string wallet_name{EnsureUniqueWalletName(request, self.MaybeArg<std::string>("wallet_name"))};
const std::string wallet_name{EnsureUniqueWalletName(request, self.MaybeArg<std::string_view>("wallet_name"))};
WalletContext& context = EnsureWalletContext(request.context);
std::shared_ptr<CWallet> wallet = GetWallet(context, wallet_name);
@ -613,7 +614,7 @@ static RPCHelpMan migratewallet()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const std::string wallet_name{EnsureUniqueWalletName(request, self.MaybeArg<std::string>("wallet_name"))};
const std::string wallet_name{EnsureUniqueWalletName(request, self.MaybeArg<std::string_view>("wallet_name"))};
SecureString wallet_pass;
wallet_pass.reserve(100);

View File

@ -17,7 +17,7 @@ static std::string TestWalletName(const std::string& endpoint, std::optional<std
{
JSONRPCRequest req;
req.URI = endpoint;
return EnsureUniqueWalletName(req, parameter ? &*parameter : nullptr);
return EnsureUniqueWalletName(req, parameter);
}
BOOST_FIXTURE_TEST_SUITE(wallet_rpc_tests, BasicTestingSetup)