Skip to content

backport: bitcoin#22764, #24505, #26074, #26039, #28230, #29277, #28597 (wallet deprecation + RPC type checking) #7242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
thepastaclaw wants to merge 8 commits into
base: develop
Choose a base branch
from
Draft

backport: bitcoin#22764, #24505, #26074, #26039, #28230, #29277, #28597 (wallet deprecation + RPC type checking) #7242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
51052e8
Merge bitcoin/bitcoin#22764: build: Include qt sources for parsing wi…
fanquake Oct 19, 2023
6eccb60
Merge bitcoin/bitcoin#24505: wallet: Add a deprecation warning for ne…
Mar 14, 2022
8a4df32
rpc: Set RPCArg options with designated initializers
Sep 13, 2022
9e4bfc1
test: Fix wrong types passed to RPCs
Dec 12, 2022
1f94e12
rpc: Run type check against RPCArgs
Dec 12, 2022
821760e
Merge bitcoin/bitcoin#28230: rpc: Add MaybeArg() and Arg() default he…
fanquake Aug 24, 2023
27fd25f
Merge bitcoin/bitcoin#29277: RPC: access RPC arguments by name
ryanofsky Apr 29, 2024
24daadc
Merge bitcoin/bitcoin#28597: wallet: No BDB creation, unless -depreca…
achow101 Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/Makefile.qt.include
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,11 +489,14 @@ SECONDARY: $(QT_QM)

$(srcdir)/qt/dashstrings.cpp: FORCE
@test -n $(XGETTEXT) || echo "xgettext is required for updating translations"
$(AM_V_GEN) cd $(srcdir); XGETTEXT=$(XGETTEXT) COPYRIGHT_HOLDERS="$(COPYRIGHT_HOLDERS)" $(PYTHON) ../share/qt/extract_strings_qt.py $(libbitcoin_node_a_SOURCES) $(libbitcoin_wallet_a_SOURCES) $(libbitcoin_common_a_SOURCES) $(libbitcoin_zmq_a_SOURCES) $(libbitcoin_consensus_a_SOURCES) $(libbitcoin_util_a_SOURCES)
$(AM_V_GEN) cd $(srcdir); XGETTEXT=$(XGETTEXT) COPYRIGHT_HOLDERS="$(COPYRIGHT_HOLDERS)" $(PYTHON) ../share/qt/extract_strings_qt.py \
$(libbitcoin_node_a_SOURCES) $(libbitcoin_wallet_a_SOURCES) $(libbitcoin_common_a_SOURCES) \
$(libbitcoin_zmq_a_SOURCES) $(libbitcoin_consensus_a_SOURCES) $(libbitcoin_util_a_SOURCES) \
$(BITCOIN_QT_BASE_CPP) $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM)

# The resulted dash_en.xlf source file should follow Transifex requirements.
# See: https://docs.transifex.com/formats/xliff#how-to-distinguish-between-a-source-file-and-a-translation-file
translate: $(srcdir)/qt/dashstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCOIN_QT_BASE_CPP) qt/bitcoin.cpp $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM)
translate: $(srcdir)/qt/dashstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCOIN_QT_BASE_CPP) $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM)
@test -n $(LUPDATE) || echo "lupdate is required for updating translations"
$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LUPDATE) -no-obsolete -I $(srcdir) -locations relative $^ -ts $(srcdir)/qt/locale/dash_en.ts
@test -n $(LCONVERT) || echo "lconvert is required for updating translations"
Expand Down
35 changes: 19 additions & 16 deletions src/rpc/blockchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,11 +500,6 @@ static RPCHelpMan getblockfrompeer()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
RPCTypeCheck(request.params, {
UniValue::VSTR, // blockhash
UniValue::VNUM, // peer_id
});

const NodeContext& node = EnsureAnyNodeContext(request.context);
ChainstateManager& chainman = EnsureChainman(node);
PeerManager& peerman = EnsurePeerman(node);
Expand Down Expand Up @@ -929,7 +924,8 @@ static RPCHelpMan getblock()
"If verbosity is 3, returns an Object with information about block <hash> and information about each transaction, including prevout information for inputs (only for unpruned blocks in the current best chain).\n",
{
{"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash"},
{"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{1}, "0 for hex-encoded data, 1 for a JSON object, 2 for JSON object with transaction data, and 3 for JSON object with transaction data including prevout information for inputs"},
{"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{1}, "0 for hex-encoded data, 1 for a JSON object, 2 for JSON object with transaction data, and 3 for JSON object with transaction data including prevout information for inputs",
RPCArgOptions{.skip_type_check = true}},
},
{
RPCResult{"for verbosity = 0",
Expand Down Expand Up @@ -1146,7 +1142,11 @@ static RPCHelpMan gettxoutsetinfo()
"Note this call may take some time if you are not using coinstatsindex.\n",
{
{"hash_type", RPCArg::Type::STR, RPCArg::Default{"hash_serialized_2"}, "Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'muhash', 'none'."},
{"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).", "", {"", "string or numeric"}},
{"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).",
RPCArgOptions{
.skip_type_check = true,
.type_str = {"", "string or numeric"},
}},
{"use_index", RPCArg::Type::BOOL, RPCArg::Default{true}, "Use coinstatsindex, if available."},
},
RPCResult{
Expand Down Expand Up @@ -2019,13 +2019,17 @@ static RPCHelpMan getblockstats()
"\nCompute per block statistics for a given window. All amounts are in duffs.\n"
"It won't work for some heights with pruning.\n",
{
{"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::NO, "The block hash or height of the target block", "", {"", "string or numeric"}},
{"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::NO, "The block hash or height of the target block",
RPCArgOptions{
.skip_type_check = true,
.type_str = {"", "string or numeric"},
}},
{"stats", RPCArg::Type::ARR, RPCArg::DefaultHint{"all values"}, "Values to plot (see result below)",
{
{"height", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Selected statistic"},
{"time", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Selected statistic"},
},
"stats"},
RPCArgOptions{.oneline_description="stats"}},
},
RPCResult{
RPCResult::Type::OBJ, "", "",
Expand Down Expand Up @@ -2457,7 +2461,7 @@ static RPCHelpMan scantxoutset()
},
},
},
"[scanobjects,...]"},
RPCArgOptions{.oneline_description="[scanobjects,...]"}},
},
{
RPCResult{"when action=='start'; only returns after scan completes", RPCResult::Type::OBJ, "", "", {
Expand Down Expand Up @@ -2497,18 +2501,17 @@ static RPCHelpMan scantxoutset()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR});

UniValue result(UniValue::VOBJ);
if (request.params[0].get_str() == "status") {
const auto action{self.Arg<std::string>("action")};
if (action == "status") {
CoinsViewScanReserver reserver;
if (reserver.reserve()) {
// no scan in progress
return UniValue::VNULL;
}
result.pushKV("progress", g_scan_progress.load());
return result;
} else if (request.params[0].get_str() == "abort") {
} else if (action == "abort") {
CoinsViewScanReserver reserver;
if (reserver.reserve()) {
// reserve was possible which means no scan was running
Expand All @@ -2517,7 +2520,7 @@ static RPCHelpMan scantxoutset()
// set the abort flag
g_should_abort_scan = true;
return true;
} else if (request.params[0].get_str() == "start") {
} else if (action == "start") {
CoinsViewScanReserver reserver;
if (!reserver.reserve()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan already in progress, use action \"abort\" or \"status\"");
Expand Down Expand Up @@ -2586,7 +2589,7 @@ static RPCHelpMan scantxoutset()
result.pushKV("unspents", unspents);
result.pushKV("total_amount", ValueFromAmount(total_in));
} else {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid command");
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid action '%s'", action));
}
return result;
},
Expand Down
60 changes: 35 additions & 25 deletions src/rpc/evo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,16 @@ static RPCArg GetRpcArg(const std::string& strParamName)
"Can be set to an empty string, which will require a ProUpServTx afterwards.",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
}}
},
RPCArgOptions{.skip_type_check = true}}
},
{"coreP2PAddrs_update",
{"coreP2PAddrs", RPCArg::Type::ARR, RPCArg::Optional::NO,
"Array of addresses in the form \"ADDR:PORT\". Must be unique on the network.",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
}}
},
RPCArgOptions{.skip_type_check = true}}
},
{"operatorKey",
{"operatorKey", RPCArg::Type::STR, RPCArg::Optional::NO,
Expand Down Expand Up @@ -187,31 +189,35 @@ static RPCArg GetRpcArg(const std::string& strParamName)
"Must be unique on the network. Can be set to an empty string, which will require a ProUpServTx afterwards.",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
}}
},
RPCArgOptions{.skip_type_check = true}}
},
{"platformP2PAddrs_update",
{"platformP2PAddrs", RPCArg::Type::ARR, RPCArg::Optional::NO,
"Array of addresses in the form \"ADDR:PORT\" used by Platform for peer-to-peer connection.\n"
"Must be unique on the network.",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
}}
},
RPCArgOptions{.skip_type_check = true}}
},
{"platformHTTPSAddrs",
{"platformHTTPSAddrs", RPCArg::Type::ARR, RPCArg::Optional::NO,
"Array of addresses in the form \"ADDR:PORT\" used by Platform for their HTTPS API.\n"
"Must be unique on the network. Can be set to an empty string, which will require a ProUpServTx afterwards.",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
}}
},
RPCArgOptions{.skip_type_check = true}}
},
{"platformHTTPSAddrs_update",
{"platformHTTPSAddrs", RPCArg::Type::ARR, RPCArg::Optional::NO,
"Array of addresses in the form \"ADDR:PORT\" used by Platform for their HTTPS API.\n"
"Must be unique on the network.",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
}}
},
RPCArgOptions{.skip_type_check = true}}
},
};

Expand Down Expand Up @@ -1408,7 +1414,8 @@ static RPCHelpMan protx_list()
" This will also include ProTx which failed PoSe verification.\n"
#endif
},
{"detailed", RPCArg::Type::BOOL, RPCArg::Default{false}, "If not specified, only the hashes of the ProTx will be returned."},
{"detailed", RPCArg::Type::BOOL, RPCArg::Default{false}, "If not specified, only the hashes of the ProTx will be returned.",
RPCArgOptions{.skip_type_check = true}},
{"height", RPCArg::Type::NUM, RPCArg::DefaultHint{"current chain-tip"}, ""},
},
RPCResult{
Expand Down Expand Up @@ -1604,26 +1611,23 @@ static const CBlockIndex* ParseBlockIndex(const UniValue& v, const ChainstateMan
{
AssertLockHeld(::cs_main);

try {
const auto hash{ParseBlock(v, chainman, strName)};
const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
if (!pindex) {
throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
}
return pindex;
} catch (...) {
// Same phrasing as ParseBlock() as it can parse heights
throw std::runtime_error(strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr()));
const auto hash{ParseBlock(v, chainman, strName)};
const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
if (!pindex) {
throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
}
return pindex;
}

static RPCHelpMan protx_diff()
{
return RPCHelpMan{"protx diff",
"\nCalculates a diff between two deterministic masternode lists. The result also contains proof data.\n",
{
{"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height."},
{"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height."},
{"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height.",
RPCArgOptions{.skip_type_check = true}},
{"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height.",
RPCArgOptions{.skip_type_check = true}},
{"extended", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Show additional fields."},
},
CSimplifiedMNListDiff::GetJsonHelp(/*key=*/"", /*optional=*/false),
Expand Down Expand Up @@ -1663,8 +1667,10 @@ static RPCHelpMan protx_listdiff()
return RPCHelpMan{"protx listdiff",
"\nCalculate a full MN list diff between two masternode lists.\n",
{
{"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height."},
{"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height."},
{"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height.",
RPCArgOptions{.skip_type_check = true}},
{"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height.",
RPCArgOptions{.skip_type_check = true}},
},
RPCResult {
RPCResult::Type::OBJ, "", "",
Expand Down Expand Up @@ -1839,8 +1845,10 @@ static RPCHelpMan evodb_verify()
"This is a read-only operation that does not modify the database.\n"
"If no heights are specified, defaults to the full range from DIP0003 activation to chain tip.\n",
{
{"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height)."},
{"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip)."},
{"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height).",
RPCArgOptions{.skip_type_check = true}},
{"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip).",
RPCArgOptions{.skip_type_check = true}},
},
RPCResult{
RPCResult::Type::OBJ, "", "",
Expand Down Expand Up @@ -1877,8 +1885,10 @@ static RPCHelpMan evodb_repair()
"If verification fails, recalculates diffs from blockchain data and replaces corrupted records.\n"
"If no heights are specified, defaults to the full range from DIP0003 activation to chain tip.\n",
{
{"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height)."},
{"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip)."},
{"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height).",
RPCArgOptions{.skip_type_check = true}},
{"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip).",
RPCArgOptions{.skip_type_check = true}},
},
RPCResult{
RPCResult::Type::OBJ, "", "",
Expand Down
4 changes: 0 additions & 4 deletions src/rpc/fees.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ static RPCHelpMan estimatesmartfee()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR});

CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context);
const NodeContext& node = EnsureAnyNodeContext(request.context);
const CTxMemPool& mempool = EnsureMemPool(node);
Expand Down Expand Up @@ -155,8 +153,6 @@ static RPCHelpMan estimaterawfee()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true);

CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context);

unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
Expand Down
14 changes: 0 additions & 14 deletions src/rpc/mempool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,6 @@ RPCHelpMan sendrawtransaction()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
RPCTypeCheck(request.params, {
UniValue::VSTR,
UniValueType(), // VNUM or VSTR, checked inside AmountFromValue()
UniValue::VBOOL,
UniValue::VBOOL,
});

CMutableTransaction mtx;
if (!DecodeHexTx(mtx, request.params[0].get_str())) {
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
Expand Down Expand Up @@ -145,10 +138,6 @@ static RPCHelpMan testmempoolaccept()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
RPCTypeCheck(request.params, {
UniValue::VARR,
UniValueType(), // VNUM or VSTR, checked inside AmountFromValue()
});
const UniValue raw_transactions = request.params[0].get_array();
if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
throw JSONRPCError(RPC_INVALID_PARAMETER,
Expand Down Expand Up @@ -809,9 +798,6 @@ static RPCHelpMan submitpackage()
if (!Params().IsMockableChain()) {
throw std::runtime_error("submitpackage is for regression testing (-regtest mode) only");
}
RPCTypeCheck(request.params, {
UniValue::VARR,
});
const UniValue raw_transactions = request.params[0].get_array();
if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
throw JSONRPCError(RPC_INVALID_PARAMETER,
Expand Down
Loading
Loading