refactor(audit): batch 7 — RPC/settings/lite/effects mechanical dedups
Six behavior-preserving consolidations from the audit: - RPCClient::call() overloads share a private performCall() (payload dump + curl_easy_perform + http code) and a static parseRpcResult() (error->RpcError extraction). The timeout overload restores the prior timeout in both the success and catch paths, exactly as before. - The six UnifiedCallback methods delegate to one splitUnified() that builds the (Callback, ErrorCallback) pair once (null-check preserved). - One liteTrimCopy() in lite_connection_service replaces 5 file-local trim-copy helpers across the lite slice. - lite_wallet_controller's inline height JSON parse now routes through the tested parseLiteHeightResponse(). - theme_effects.cpp: unpackRGB()/scaledAlpha() replace 14 RGB-unpack + 5 alpha-scale copy-paste blocks. - settings.cpp load() uses loadScalar()/loadClamped() helpers for ~44 scalar fields. Besides removing boilerplate this HARDENS loading: ~44 fields that previously only checked contains() now also verify the JSON type, so a malformed value in settings.json falls back to the default instead of throwing/misreading. Custom cases (enum parses, legacy migrations, arrays) stay inline; save() is unchanged. Full-node + Lite build clean; ctest 1/1; hygiene clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -290,13 +290,8 @@ json RPCClient::makePayload(const std::string& method, const json& params)
|
||||
};
|
||||
}
|
||||
|
||||
json RPCClient::call(const std::string& method, const json& params)
|
||||
std::string RPCClient::performCall(const std::string& method, const json& params, long& httpCodeOut)
|
||||
{
|
||||
std::lock_guard<std::recursive_mutex> lk(curl_mutex_);
|
||||
if (!impl_->curl) {
|
||||
throw std::runtime_error("Not connected");
|
||||
}
|
||||
|
||||
emitRpcTrace(method);
|
||||
|
||||
json payload = makePayload(method, params);
|
||||
@@ -310,22 +305,27 @@ json RPCClient::call(const std::string& method, const json& params)
|
||||
|
||||
// Perform request
|
||||
CURLcode res = curl_easy_perform(impl_->curl);
|
||||
|
||||
|
||||
if (res != CURLE_OK) {
|
||||
throw std::runtime_error("RPC request failed: " + std::string(curl_easy_strerror(res)));
|
||||
}
|
||||
|
||||
// Check HTTP response code
|
||||
long http_code = 0;
|
||||
curl_easy_getinfo(impl_->curl, CURLINFO_RESPONSE_CODE, &http_code);
|
||||
|
||||
httpCodeOut = 0;
|
||||
curl_easy_getinfo(impl_->curl, CURLINFO_RESPONSE_CODE, &httpCodeOut);
|
||||
|
||||
return response_data;
|
||||
}
|
||||
|
||||
json RPCClient::parseRpcResult(long httpCode, const std::string& body)
|
||||
{
|
||||
// Bitcoin/Hush RPC returns HTTP 500 for application-level errors
|
||||
// (insufficient funds, bad params, etc.) with a valid JSON body.
|
||||
// Parse the body first to extract the real error message.
|
||||
if (http_code != 200) {
|
||||
if (httpCode != 200) {
|
||||
int errCode = 0;
|
||||
try {
|
||||
json response = json::parse(response_data);
|
||||
json response = json::parse(body);
|
||||
if (response.contains("error") && response["error"].is_object()) {
|
||||
if (response["error"].contains("code") && response["error"]["code"].is_number_integer())
|
||||
errCode = response["error"]["code"].get<int>();
|
||||
@@ -337,10 +337,10 @@ json RPCClient::call(const std::string& method, const json& params)
|
||||
} catch (const json::exception&) {
|
||||
// Body wasn't valid JSON — fall through to generic HTTP error
|
||||
}
|
||||
throw RpcError(errCode, "RPC error: HTTP " + std::to_string(http_code));
|
||||
throw RpcError(errCode, "RPC error: HTTP " + std::to_string(httpCode));
|
||||
}
|
||||
|
||||
json response = json::parse(response_data);
|
||||
json response = json::parse(body);
|
||||
|
||||
if (response.contains("error") && !response["error"].is_null()) {
|
||||
int errCode = 0;
|
||||
@@ -358,6 +358,18 @@ json RPCClient::call(const std::string& method, const json& params)
|
||||
return response["result"];
|
||||
}
|
||||
|
||||
json RPCClient::call(const std::string& method, const json& params)
|
||||
{
|
||||
std::lock_guard<std::recursive_mutex> lk(curl_mutex_);
|
||||
if (!impl_->curl) {
|
||||
throw std::runtime_error("Not connected");
|
||||
}
|
||||
|
||||
long http_code = 0;
|
||||
std::string response_data = performCall(method, params, http_code);
|
||||
return parseRpcResult(http_code, response_data);
|
||||
}
|
||||
|
||||
json RPCClient::call(const std::string& method, const json& params, long timeoutSec)
|
||||
{
|
||||
std::lock_guard<std::recursive_mutex> lk(curl_mutex_);
|
||||
@@ -365,65 +377,16 @@ json RPCClient::call(const std::string& method, const json& params, long timeout
|
||||
throw std::runtime_error("Not connected");
|
||||
}
|
||||
|
||||
emitRpcTrace(method);
|
||||
|
||||
// Temporarily override timeout
|
||||
long prevTimeout = 30L;
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_TIMEOUT, timeoutSec);
|
||||
|
||||
try {
|
||||
// Unlock before calling to avoid recursive lock issues — but we already hold it,
|
||||
// and call() also locks with recursive_mutex, so just delegate to the body directly.
|
||||
json payload = makePayload(method, params);
|
||||
std::string body = payload.dump();
|
||||
std::string response_data;
|
||||
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_POSTFIELDS, body.c_str());
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_POSTFIELDSIZE, (long)body.size());
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_WRITEDATA, &response_data);
|
||||
|
||||
CURLcode res = curl_easy_perform(impl_->curl);
|
||||
|
||||
long http_code = 0;
|
||||
std::string response_data = performCall(method, params, http_code);
|
||||
// Restore original timeout
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_TIMEOUT, prevTimeout);
|
||||
|
||||
if (res != CURLE_OK) {
|
||||
throw std::runtime_error("RPC request failed: " + std::string(curl_easy_strerror(res)));
|
||||
}
|
||||
|
||||
long http_code = 0;
|
||||
curl_easy_getinfo(impl_->curl, CURLINFO_RESPONSE_CODE, &http_code);
|
||||
|
||||
if (http_code != 200) {
|
||||
int errCode = 0;
|
||||
try {
|
||||
json response = json::parse(response_data);
|
||||
if (response.contains("error") && response["error"].is_object()) {
|
||||
if (response["error"].contains("code") && response["error"]["code"].is_number_integer())
|
||||
errCode = response["error"]["code"].get<int>();
|
||||
if (response["error"].contains("message") && response["error"]["message"].is_string())
|
||||
throw RpcError(errCode, response["error"]["message"].get<std::string>());
|
||||
throw RpcError(errCode, "RPC error: " + response["error"].dump());
|
||||
}
|
||||
} catch (const json::exception&) {}
|
||||
throw RpcError(errCode, "RPC error: HTTP " + std::to_string(http_code));
|
||||
}
|
||||
|
||||
json response = json::parse(response_data);
|
||||
if (response.contains("error") && !response["error"].is_null()) {
|
||||
int errCode = 0;
|
||||
std::string err_msg;
|
||||
if (response["error"].is_object()) {
|
||||
if (response["error"].contains("code") && response["error"]["code"].is_number_integer())
|
||||
errCode = response["error"]["code"].get<int>();
|
||||
if (response["error"].contains("message") && response["error"]["message"].is_string())
|
||||
err_msg = response["error"]["message"].get<std::string>();
|
||||
}
|
||||
if (err_msg.empty()) err_msg = response["error"].dump();
|
||||
throw RpcError(errCode, "RPC error: " + err_msg);
|
||||
}
|
||||
|
||||
return response["result"];
|
||||
return parseRpcResult(http_code, response_data);
|
||||
} catch (...) {
|
||||
// Ensure timeout is always restored
|
||||
curl_easy_setopt(impl_->curl, CURLOPT_TIMEOUT, prevTimeout);
|
||||
@@ -762,43 +725,40 @@ void RPCClient::z_listReceivedByAddress(const std::string& address, int minconf,
|
||||
doRPC("z_listreceivedbyaddress", {address, minconf}, cb, err);
|
||||
}
|
||||
|
||||
// Unified callback versions
|
||||
void RPCClient::getInfo(UnifiedCallback cb)
|
||||
// Adapts a UnifiedCallback into the (Callback, ErrorCallback) pair doRPC expects: the
|
||||
// success lambda forwards (result, ""), the error lambda forwards ({}, error). The cb
|
||||
// null-check is preserved on each invocation.
|
||||
std::pair<Callback, ErrorCallback> RPCClient::splitUnified(UnifiedCallback cb)
|
||||
{
|
||||
doRPC("getinfo", {},
|
||||
return {
|
||||
[cb](const json& result) {
|
||||
if (cb) cb(result, "");
|
||||
},
|
||||
[cb](const std::string& error) {
|
||||
if (cb) cb(json{}, error);
|
||||
}
|
||||
);
|
||||
};
|
||||
}
|
||||
|
||||
// Unified callback versions
|
||||
void RPCClient::getInfo(UnifiedCallback cb)
|
||||
{
|
||||
auto handlers = splitUnified(std::move(cb));
|
||||
doRPC("getinfo", {}, handlers.first, handlers.second);
|
||||
}
|
||||
|
||||
void RPCClient::rescanBlockchain(int startHeight, UnifiedCallback cb)
|
||||
{
|
||||
// hush/komodo daemons expose this as "rescan <height>", not bitcoin's "rescanblockchain".
|
||||
doRPC("rescan", {startHeight},
|
||||
[cb](const json& result) {
|
||||
if (cb) cb(result, "");
|
||||
},
|
||||
[cb](const std::string& error) {
|
||||
if (cb) cb(json{}, error);
|
||||
}
|
||||
);
|
||||
auto handlers = splitUnified(std::move(cb));
|
||||
doRPC("rescan", {startHeight}, handlers.first, handlers.second);
|
||||
}
|
||||
|
||||
void RPCClient::z_shieldCoinbase(const std::string& fromAddr, const std::string& toAddr,
|
||||
double fee, int limit, UnifiedCallback cb)
|
||||
{
|
||||
doRPC("z_shieldcoinbase", {fromAddr, toAddr, fee, limit},
|
||||
[cb](const json& result) {
|
||||
if (cb) cb(result, "");
|
||||
},
|
||||
[cb](const std::string& error) {
|
||||
if (cb) cb(json{}, error);
|
||||
}
|
||||
);
|
||||
auto handlers = splitUnified(std::move(cb));
|
||||
doRPC("z_shieldcoinbase", {fromAddr, toAddr, fee, limit}, handlers.first, handlers.second);
|
||||
}
|
||||
|
||||
void RPCClient::z_mergeToAddress(const std::vector<std::string>& fromAddrs, const std::string& toAddr,
|
||||
@@ -808,14 +768,8 @@ void RPCClient::z_mergeToAddress(const std::vector<std::string>& fromAddrs, cons
|
||||
for (const auto& addr : fromAddrs) {
|
||||
addrs.push_back(addr);
|
||||
}
|
||||
doRPC("z_mergetoaddress", {addrs, toAddr, fee, 0, limit},
|
||||
[cb](const json& result) {
|
||||
if (cb) cb(result, "");
|
||||
},
|
||||
[cb](const std::string& error) {
|
||||
if (cb) cb(json{}, error);
|
||||
}
|
||||
);
|
||||
auto handlers = splitUnified(std::move(cb));
|
||||
doRPC("z_mergetoaddress", {addrs, toAddr, fee, 0, limit}, handlers.first, handlers.second);
|
||||
}
|
||||
|
||||
void RPCClient::z_getOperationStatus(const std::vector<std::string>& opids, UnifiedCallback cb)
|
||||
@@ -824,30 +778,18 @@ void RPCClient::z_getOperationStatus(const std::vector<std::string>& opids, Unif
|
||||
for (const auto& id : opids) {
|
||||
ids.push_back(id);
|
||||
}
|
||||
doRPC("z_getoperationstatus", {ids},
|
||||
[cb](const json& result) {
|
||||
if (cb) cb(result, "");
|
||||
},
|
||||
[cb](const std::string& error) {
|
||||
if (cb) cb(json{}, error);
|
||||
}
|
||||
);
|
||||
auto handlers = splitUnified(std::move(cb));
|
||||
doRPC("z_getoperationstatus", {ids}, handlers.first, handlers.second);
|
||||
}
|
||||
|
||||
void RPCClient::getBlock(int height, UnifiedCallback cb)
|
||||
{
|
||||
// First get block hash, then get block
|
||||
getBlockHash(height,
|
||||
getBlockHash(height,
|
||||
[this, cb](const json& hashResult) {
|
||||
std::string hash = hashResult.get<std::string>();
|
||||
getBlock(hash,
|
||||
[cb](const json& blockResult) {
|
||||
if (cb) cb(blockResult, "");
|
||||
},
|
||||
[cb](const std::string& error) {
|
||||
if (cb) cb(json{}, error);
|
||||
}
|
||||
);
|
||||
auto handlers = splitUnified(cb);
|
||||
getBlock(hash, handlers.first, handlers.second);
|
||||
},
|
||||
[cb](const std::string& error) {
|
||||
if (cb) cb(json{}, error);
|
||||
|
||||
@@ -245,6 +245,17 @@ private:
|
||||
json makePayload(const std::string& method, const json& params = json::array());
|
||||
void doRPC(const std::string& method, const json& params, Callback cb, ErrorCallback err);
|
||||
|
||||
// Shared blocking-call core: dumps the payload, performs the request (throwing on a
|
||||
// curl transport error) and returns the response body plus HTTP status. Caller must
|
||||
// hold curl_mutex_ and have verified impl_->curl.
|
||||
std::string performCall(const std::string& method, const json& params, long& httpCodeOut);
|
||||
// Centralizes the HTTP-code check and JSON error->RpcError extraction, returning
|
||||
// response["result"] on success.
|
||||
static json parseRpcResult(long httpCode, const std::string& body);
|
||||
|
||||
// Splits a UnifiedCallback into the (Callback, ErrorCallback) pair used by doRPC.
|
||||
static std::pair<Callback, ErrorCallback> splitUnified(UnifiedCallback cb);
|
||||
|
||||
std::string host_;
|
||||
std::string port_;
|
||||
std::string auth_; // Base64 encoded "user:password"
|
||||
|
||||
Reference in New Issue
Block a user