Merge bitcoin/bitcoin#34113: refactor: [rpc] Remove confusing and brittle integral casts

fa66e2d07a refactor: [rpc] Remove confusing and brittle integral casts (MarcoFalke)

Pull request description:

  When constructing an UniValue from integral values, historically (long ago), in some cases casts where needed. With the current UniValue constructor, only very few are actually needed.

  Keeping the unused casts around is:

  * confusing, because code readers do not understand why they are needed
  * brittle, because some may copy them into new places, where they will lead to hard-to-find logic bugs, such as the ones fixed in pull https://github.com/bitcoin/bitcoin/pull/34112

  So fix all issues by removing them, except for a few cases, where casting was required:
  * `ret.pushKV("coinbase", static_cast<bool>(coin->fCoinBase));`, or
  * `static_cast<std::underlying_type_t<decltype(info.nServices)>>(info.nServices)`.

  This hardening refactor does not fix any bugs and does not change any behavior.

ACKs for top commit:
  sedited:
    ACK fa66e2d07a
  rkrux:
    ACK fa66e2d07a

Tree-SHA512: 13c9c59ad021ea03cdabe10d58850cef96d792634c499e62227cc2e7e5cace066ebd9a8ef3f979eaba98cadf8a525c6e6df909a07115559c0450bd9fc3a9763e
This commit is contained in:
merge-script
2025-12-27 16:35:21 +00:00
9 changed files with 36 additions and 36 deletions

View File

@@ -1084,10 +1084,10 @@ static RPCHelpMan gettxoutsetinfo()
const std::optional<CCoinsStats> maybe_stats = GetUTXOStats(coins_view, *blockman, hash_type, node.rpc_interruption_point, pindex, index_requested);
if (maybe_stats.has_value()) {
const CCoinsStats& stats = maybe_stats.value();
ret.pushKV("height", (int64_t)stats.nHeight);
ret.pushKV("height", stats.nHeight);
ret.pushKV("bestblock", stats.hashBlock.GetHex());
ret.pushKV("txouts", (int64_t)stats.nTransactionOutputs);
ret.pushKV("bogosize", (int64_t)stats.nBogoSize);
ret.pushKV("txouts", stats.nTransactionOutputs);
ret.pushKV("bogosize", stats.nBogoSize);
if (hash_type == CoinStatsHashType::HASH_SERIALIZED) {
ret.pushKV("hash_serialized_3", stats.hashSerialized.GetHex());
}
@@ -1217,13 +1217,13 @@ static RPCHelpMan gettxout()
if (coin->nHeight == MEMPOOL_HEIGHT) {
ret.pushKV("confirmations", 0);
} else {
ret.pushKV("confirmations", (int64_t)(pindex->nHeight - coin->nHeight + 1));
ret.pushKV("confirmations", pindex->nHeight - coin->nHeight + 1);
}
ret.pushKV("value", ValueFromAmount(coin->out.nValue));
UniValue o(UniValue::VOBJ);
ScriptToUniv(coin->out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true);
ret.pushKV("scriptPubKey", std::move(o));
ret.pushKV("coinbase", (bool)coin->fCoinBase);
ret.pushKV("coinbase", static_cast<bool>(coin->fCoinBase));
return ret;
},
@@ -1823,7 +1823,7 @@ static RPCHelpMan getchaintxstats()
const int64_t nTimeDiff{pindex->GetMedianTimePast() - past_block.GetMedianTimePast()};
UniValue ret(UniValue::VOBJ);
ret.pushKV("time", (int64_t)pindex->nTime);
ret.pushKV("time", pindex->nTime);
if (pindex->m_chain_tx_count) {
ret.pushKV("txcount", pindex->m_chain_tx_count);
}
@@ -2119,7 +2119,7 @@ static RPCHelpMan getblockstats()
ret_all.pushKV("avgtxsize", (block.vtx.size() > 1) ? total_size / (block.vtx.size() - 1) : 0);
ret_all.pushKV("blockhash", pindex.GetBlockHash().GetHex());
ret_all.pushKV("feerate_percentiles", std::move(feerates_res));
ret_all.pushKV("height", (int64_t)pindex.nHeight);
ret_all.pushKV("height", pindex.nHeight);
ret_all.pushKV("ins", inputs);
ret_all.pushKV("maxfee", maxfee);
ret_all.pushKV("maxfeerate", maxfeerate);
@@ -2140,7 +2140,7 @@ static RPCHelpMan getblockstats()
ret_all.pushKV("total_size", total_size);
ret_all.pushKV("total_weight", total_weight);
ret_all.pushKV("totalfee", totalfee);
ret_all.pushKV("txs", (int64_t)block.vtx.size());
ret_all.pushKV("txs", block.vtx.size());
ret_all.pushKV("utxo_increase", outputs - inputs);
ret_all.pushKV("utxo_size_inc", utxo_size_inc);
ret_all.pushKV("utxo_increase_actual", utxos - inputs);
@@ -3437,7 +3437,7 @@ return RPCHelpMan{
const CChain& chain = cs.m_chain;
const CBlockIndex* tip = chain.Tip();
data.pushKV("blocks", (int)chain.Height());
data.pushKV("blocks", chain.Height());
data.pushKV("bestblockhash", tip->GetBlockHash().GetHex());
data.pushKV("bits", strprintf("%08x", tip->nBits));
data.pushKV("target", GetTarget(*tip, chainman.GetConsensus().powLimit).GetHex());

View File

@@ -195,14 +195,14 @@ static RPCHelpMan estimaterawfee()
if (feeRate != CFeeRate(0)) {
horizon_result.pushKV("feerate", ValueFromAmount(feeRate.GetFeePerK()));
horizon_result.pushKV("decay", buckets.decay);
horizon_result.pushKV("scale", (int)buckets.scale);
horizon_result.pushKV("scale", buckets.scale);
horizon_result.pushKV("pass", std::move(passbucket));
// buckets.fail.start == -1 indicates that all buckets passed, there is no fail bucket to output
if (buckets.fail.start != -1) horizon_result.pushKV("fail", std::move(failbucket));
} else {
// Output only information that is still meaningful in the event of error
horizon_result.pushKV("decay", buckets.decay);
horizon_result.pushKV("scale", (int)buckets.scale);
horizon_result.pushKV("scale", buckets.scale);
horizon_result.pushKV("fail", std::move(failbucket));
errors.push_back("Insufficient data or no feerate found which meets threshold");
horizon_result.pushKV("errors", std::move(errors));

View File

@@ -333,7 +333,7 @@ static void clusterToJSON(const CTxMemPool& pool, UniValue& info, std::vector<co
total_weight += tx->GetAdjustedWeight();
}
info.pushKV("clusterweight", total_weight);
info.pushKV("txcount", (int)cluster.size());
info.pushKV("txcount", cluster.size());
// Output the cluster by chunk. This isn't handed to us by the mempool, but
// we can calculate it by looking at the chunk feerates of each transaction
@@ -366,10 +366,10 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
auto [ancestor_count, ancestor_size, ancestor_fees] = pool.CalculateAncestorData(e);
auto [descendant_count, descendant_size, descendant_fees] = pool.CalculateDescendantData(e);
info.pushKV("vsize", (int)e.GetTxSize());
info.pushKV("weight", (int)e.GetTxWeight());
info.pushKV("vsize", e.GetTxSize());
info.pushKV("weight", e.GetTxWeight());
info.pushKV("time", count_seconds(e.GetTime()));
info.pushKV("height", (int)e.GetHeight());
info.pushKV("height", e.GetHeight());
info.pushKV("descendantcount", descendant_count);
info.pushKV("descendantsize", descendant_size);
info.pushKV("ancestorcount", ancestor_count);
@@ -815,7 +815,7 @@ static RPCHelpMan gettxspendingprevout()
for (const COutPoint& prevout : prevouts) {
UniValue o(UniValue::VOBJ);
o.pushKV("txid", prevout.hash.ToString());
o.pushKV("vout", (uint64_t)prevout.n);
o.pushKV("vout", prevout.n);
const CTransaction* spendingTx = mempool.GetConflictTx(prevout);
if (spendingTx != nullptr) {
@@ -836,15 +836,15 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool)
LOCK(pool.cs);
UniValue ret(UniValue::VOBJ);
ret.pushKV("loaded", pool.GetLoadTried());
ret.pushKV("size", (int64_t)pool.size());
ret.pushKV("bytes", (int64_t)pool.GetTotalTxSize());
ret.pushKV("usage", (int64_t)pool.DynamicMemoryUsage());
ret.pushKV("size", pool.size());
ret.pushKV("bytes", pool.GetTotalTxSize());
ret.pushKV("usage", pool.DynamicMemoryUsage());
ret.pushKV("total_fee", ValueFromAmount(pool.GetTotalFee()));
ret.pushKV("maxmempool", pool.m_opts.max_size_bytes);
ret.pushKV("mempoolminfee", ValueFromAmount(std::max(pool.GetMinFee(), pool.m_opts.min_relay_feerate).GetFeePerK()));
ret.pushKV("minrelaytxfee", ValueFromAmount(pool.m_opts.min_relay_feerate.GetFeePerK()));
ret.pushKV("incrementalrelayfee", ValueFromAmount(pool.m_opts.incremental_relay_feerate.GetFeePerK()));
ret.pushKV("unbroadcastcount", uint64_t{pool.GetUnbroadcastTxs().size()});
ret.pushKV("unbroadcastcount", pool.GetUnbroadcastTxs().size());
ret.pushKV("fullrbf", true);
ret.pushKV("permitbaremultisig", pool.m_opts.permit_bare_multisig);
ret.pushKV("maxdatacarriersize", pool.m_opts.max_datacarrier_bytes.value_or(0));
@@ -1270,7 +1270,7 @@ static RPCHelpMan submitpackage()
break;
case MempoolAcceptResult::ResultType::VALID:
case MempoolAcceptResult::ResultType::MEMPOOL_ENTRY:
result_inner.pushKV("vsize", int64_t{it->second.m_vsize.value()});
result_inner.pushKV("vsize", it->second.m_vsize.value());
UniValue fees(UniValue::VOBJ);
fees.pushKV("base", ValueFromAmount(it->second.m_base_fees.value()));
if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) {

View File

@@ -469,7 +469,7 @@ static RPCHelpMan getmininginfo()
obj.pushKV("difficulty", GetDifficulty(tip));
obj.pushKV("target", GetTarget(tip, chainman.GetConsensus().powLimit).GetHex());
obj.pushKV("networkhashps", getnetworkhashps().HandleRequest(request));
obj.pushKV("pooledtx", (uint64_t)mempool.size());
obj.pushKV("pooledtx", mempool.size());
BlockAssembler::Options assembler_options;
ApplyArgsManOptions(*node.args, assembler_options);
obj.pushKV("blockmintxfee", ValueFromAmount(assembler_options.blockMinFeeRate.GetFeePerK()));
@@ -988,7 +988,7 @@ static RPCHelpMan getblocktemplate()
result.pushKV("previousblockhash", block.hashPrevBlock.GetHex());
result.pushKV("transactions", std::move(transactions));
result.pushKV("coinbaseaux", std::move(aux));
result.pushKV("coinbasevalue", (int64_t)block.vtx[0]->vout[0].nValue);
result.pushKV("coinbasevalue", block.vtx[0]->vout[0].nValue);
result.pushKV("longpollid", tip.GetHex() + ToString(nTransactionsUpdatedLast));
result.pushKV("target", hashTarget.GetHex());
result.pushKV("mintime", GetMinimumTime(pindexPrev, consensusParams.DifficultyAdjustmentInterval()));
@@ -1005,11 +1005,11 @@ static RPCHelpMan getblocktemplate()
result.pushKV("sigoplimit", nSigOpLimit);
result.pushKV("sizelimit", nSizeLimit);
if (!fPreSegWit) {
result.pushKV("weightlimit", (int64_t)MAX_BLOCK_WEIGHT);
result.pushKV("weightlimit", MAX_BLOCK_WEIGHT);
}
result.pushKV("curtime", block.GetBlockTime());
result.pushKV("bits", strprintf("%08x", block.nBits));
result.pushKV("height", (int64_t)(pindexPrev->nHeight+1));
result.pushKV("height", pindexPrev->nHeight + 1);
if (consensusParams.signet_blocks) {
result.pushKV("signet_challenge", HexStr(consensusParams.signet_challenge));

View File

@@ -956,7 +956,7 @@ static RPCHelpMan getnodeaddresses()
for (const CAddress& addr : vAddr) {
UniValue obj(UniValue::VOBJ);
obj.pushKV("time", int64_t{TicksSinceEpoch<std::chrono::seconds>(addr.nTime)});
obj.pushKV("services", (uint64_t)addr.nServices);
obj.pushKV("services", static_cast<std::underlying_type_t<decltype(addr.nServices)>>(addr.nServices));
obj.pushKV("address", addr.ToStringAddr());
obj.pushKV("port", addr.GetPort());
obj.pushKV("network", GetNetworkName(addr.GetNetClass()));
@@ -1123,7 +1123,7 @@ UniValue AddrmanEntryToJSON(const AddrInfo& info, const CConnman& connman)
ret.pushKV("mapped_as", mapped_as);
}
ret.pushKV("port", info.GetPort());
ret.pushKV("services", (uint64_t)info.nServices);
ret.pushKV("services", static_cast<std::underlying_type_t<decltype(info.nServices)>>(info.nServices));
ret.pushKV("time", int64_t{TicksSinceEpoch<std::chrono::seconds>(info.nTime)});
ret.pushKV("network", GetNetworkName(info.GetNetClass()));
ret.pushKV("source", info.source.ToStringAddr());

View File

@@ -1428,8 +1428,8 @@ static RPCHelpMan decodepsbt()
UniValue tree(UniValue::VARR);
for (const auto& [depth, leaf_ver, script] : output.m_tap_tree) {
UniValue elem(UniValue::VOBJ);
elem.pushKV("depth", (int)depth);
elem.pushKV("leaf_ver", (int)leaf_ver);
elem.pushKV("depth", depth);
elem.pushKV("leaf_ver", leaf_ver);
elem.pushKV("script", HexStr(script));
tree.push_back(std::move(elem));
}
@@ -1971,7 +1971,7 @@ static RPCHelpMan analyzepsbt()
if (!inputs_result.empty()) result.pushKV("inputs", std::move(inputs_result));
if (psbta.estimated_vsize != std::nullopt) {
result.pushKV("estimated_vsize", (int)*psbta.estimated_vsize);
result.pushKV("estimated_vsize", *psbta.estimated_vsize);
}
if (psbta.estimated_feerate != std::nullopt) {
result.pushKV("estimated_feerate", ValueFromAmount(psbta.estimated_feerate->GetFeePerK()));

View File

@@ -175,14 +175,14 @@ static void TxInErrorToJSON(const CTxIn& txin, UniValue& vErrorsRet, const std::
{
UniValue entry(UniValue::VOBJ);
entry.pushKV("txid", txin.prevout.hash.ToString());
entry.pushKV("vout", (uint64_t)txin.prevout.n);
entry.pushKV("vout", txin.prevout.n);
UniValue witness(UniValue::VARR);
for (unsigned int i = 0; i < txin.scriptWitness.stack.size(); i++) {
witness.push_back(HexStr(txin.scriptWitness.stack[i]));
}
entry.pushKV("witness", std::move(witness));
entry.pushKV("scriptSig", HexStr(txin.scriptSig));
entry.pushKV("sequence", (uint64_t)txin.nSequence);
entry.pushKV("sequence", txin.nSequence);
entry.pushKV("error", strMessage);
vErrorsRet.push_back(std::move(entry));
}

View File

@@ -389,7 +389,7 @@ RPCHelpMan listlockunspent()
UniValue o(UniValue::VOBJ);
o.pushKV("txid", outpt.hash.GetHex());
o.pushKV("vout", (int)outpt.n);
o.pushKV("vout", outpt.n);
ret.push_back(std::move(o));
}
@@ -617,7 +617,7 @@ RPCHelpMan listunspent()
UniValue entry(UniValue::VOBJ);
entry.pushKV("txid", out.outpoint.hash.GetHex());
entry.pushKV("vout", (int)out.outpoint.n);
entry.pushKV("vout", out.outpoint.n);
if (fValidAddress) {
entry.pushKV("address", EncodeDestination(address));

View File

@@ -89,8 +89,8 @@ static RPCHelpMan getwalletinfo()
obj.pushKV("walletname", pwallet->GetName());
obj.pushKV("walletversion", latest_legacy_wallet_minversion);
obj.pushKV("format", pwallet->GetDatabase().Format());
obj.pushKV("txcount", (int)pwallet->mapWallet.size());
obj.pushKV("keypoolsize", (int64_t)kpExternalSize);
obj.pushKV("txcount", pwallet->mapWallet.size());
obj.pushKV("keypoolsize", kpExternalSize);
obj.pushKV("keypoolsize_hd_internal", pwallet->GetKeyPoolSize() - kpExternalSize);
if (pwallet->HasEncryptionKeys()) {