diff --git a/ci/dash/lint-tidy.sh b/ci/dash/lint-tidy.sh index e82444291d5e..3dbb26c16050 100755 --- a/ci/dash/lint-tidy.sh +++ b/ci/dash/lint-tidy.sh @@ -94,45 +94,16 @@ python3 "${CLANG_TIDY_CACHE_PY}" --show-stats 2>&1 || true cleanup_ctcache "${CTCACHE_DIR}" "${CTCACHE_MAXSIZE_MB}" echo "==========================" +# Filter out files by regex here, because regex may not be +# accepted in src/.bear-tidy-config +# Filter out: +# * qt qrc and moc generated files +# * secp256k1 +jq 'map(select(.file | test("src/qt/qrc_.*\\.cpp$|/moc_.*\\.cpp$|src/secp256k1/src/") | not))' ../compile_commands.json > tmp.json +mv tmp.json ../compile_commands.json + cd "${BASE_ROOT_DIR}/build-ci/dashcore-${BUILD_TARGET}" iwyu_tool.py \ - "src/common/init.cpp" \ - "src/common/url.cpp" \ - "src/compat" \ - "src/dbwrapper.cpp" \ - "src/init" \ - "src/kernel" \ - "src/node/chainstate.cpp" \ - "src/node/chainstatemanager_args.cpp" \ - "src/node/mempool_args.cpp" \ - "src/node/utxo_snapshot.cpp" \ - "src/node/validation_cache_args.cpp" \ - "src/node/minisketchwrapper.cpp" \ - "src/policy/feerate.cpp" \ - "src/policy/packages.cpp" \ - "src/policy/settings.cpp" \ - "src/primitives/transaction.cpp" \ - "src/random.cpp" \ - "src/rpc/fees.cpp" \ - "src/rpc/signmessage.cpp" \ - "src/test/fuzz/string.cpp" \ - "src/test/fuzz/txorphan.cpp" \ - "src/test/fuzz/util/" \ - "src/util/bip32.cpp" \ - "src/util/bytevectorhash.cpp" \ - "src/util/check.cpp" \ - "src/util/error.cpp" \ - "src/util/exception.cpp" \ - "src/util/hasher.cpp" \ - "src/util/message.cpp" \ - "src/util/moneystr.cpp" \ - "src/util/serfloat.cpp" \ - "src/util/spanparsing.cpp" \ - "src/util/string.cpp" \ - "src/util/strencodings.cpp" \ - "src/util/syserror.cpp" \ - "src/util/threadinterrupt.cpp" \ - "src/zmq" \ -p . "${MAKEJOBS}" \ -- -Xiwyu --cxx17ns -Xiwyu --mapping_file="${BASE_ROOT_DIR}/contrib/devtools/iwyu/bitcoin.core.imp" \ -Xiwyu --max_line_length=160 \ diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index 02005530af72..cc1b740c6098 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -36,7 +36,7 @@ python3 --version # NOTE: BUMP ALSO contrib/containers/ci/ci-slim.Dockerfile ${CI_RETRY_EXE} pip3 install \ codespell==2.2.1 \ - flake8==5.0.4 \ + flake8==6.1.0 \ lief==0.13.2 \ mypy==0.981 \ pyzmq==24.0.1 \ diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh index e71b490a4018..21e8032576f8 100755 --- a/ci/test/00_setup_env.sh +++ b/ci/test/00_setup_env.sh @@ -6,6 +6,8 @@ export LC_ALL=C.UTF-8 +set -ex + # The root dir. # The ci system copies this folder. BASE_ROOT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )"/../../ >/dev/null 2>&1 && pwd ) @@ -52,7 +54,6 @@ export RUN_STDLIB_PATH_CHECK=${RUN_STDLIB_PATH_CHECK:-false} export TEST_RUNNER_TIMEOUT_FACTOR=${TEST_RUNNER_TIMEOUT_FACTOR:-4} export RUN_FUZZ_TESTS=${RUN_FUZZ_TESTS:-false} -export CONTAINER_NAME=${CONTAINER_NAME:-ci_unnamed} # Randomize test order. # See https://www.boost.org/doc/libs/1_71_0/libs/test/doc/html/boost_test/utf_reference/rt_param_reference/random.html export BOOST_TEST_RANDOM=${BOOST_TEST_RANDOM:-1} diff --git a/ci/test/00_setup_env_mac_native.sh b/ci/test/00_setup_env_mac_native.sh index 9241fe348f55..c3edd52d499b 100755 --- a/ci/test/00_setup_env_mac_native.sh +++ b/ci/test/00_setup_env_mac_native.sh @@ -15,3 +15,5 @@ export CI_OS_NAME="macos" export NO_DEPENDS=1 export OSX_SDK="" export CCACHE_MAXSIZE=400M +export RUN_FUZZ_TESTS=true +export FUZZ_TESTS_CONFIG="--exclude banman" # https://github.com/bitcoin/bitcoin/issues/27924 diff --git a/contrib/containers/ci/ci-slim.Dockerfile b/contrib/containers/ci/ci-slim.Dockerfile index a9701c81d0ff..2cce5c2c705c 100644 --- a/contrib/containers/ci/ci-slim.Dockerfile +++ b/contrib/containers/ci/ci-slim.Dockerfile @@ -42,6 +42,7 @@ RUN set -ex; \ curl \ g++ \ git \ + jq \ libbz2-dev \ liblzma-dev \ libreadline-dev \ @@ -79,7 +80,7 @@ ENV UV_SYSTEM_PYTHON=1 # NOTE: if versions are changed, update ci/lint/04_install.sh RUN uv pip install --system --break-system-packages \ codespell==2.2.1 \ - flake8==5.0.4 \ + flake8==6.1.0 \ jinja2 \ lief==0.13.2 \ mypy==0.981 \ diff --git a/doc/release-notes-26485.md b/doc/release-notes-26485.md new file mode 100644 index 000000000000..e17a36f49f97 --- /dev/null +++ b/doc/release-notes-26485.md @@ -0,0 +1,16 @@ +JSON-RPC +--- + +For RPC methods which accept `options` parameters ((`importmulti`, `listunspent`, `fundrawtransaction`, `send`, `sendall`, `walletcreatefundedpsbt`, `simulaterawtransaction`), it is now possible to pass the options as named parameters without the need for a nested object. (#8010) + +This means it is possible make calls like: + +```sh +src/dash-cli -named listunspent coinType=1 +``` + +instead of + +```sh +src/dash-cli -named listunspent query_options='{"coinType": 1}' +``` diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp index 17c249641518..67b1dd1d904d 100644 --- a/src/bench/wallet_create_tx.cpp +++ b/src/bench/wallet_create_tx.cpp @@ -84,6 +84,8 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type { const auto test_setup = MakeNoLogFileContext(); + // Set clock to genesis block, so the descriptors/keys creation time don't interfere with the blocks scanning process. + SetMockTime(test_setup->m_node.chainman->GetParams().GenesisBlock().nTime); CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()}; { LOCK(wallet.cs_wallet); @@ -137,6 +139,8 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type static void AvailableCoins(benchmark::Bench& bench, const std::vector& output_type) { const auto test_setup = MakeNoLogFileContext(); + // Set clock to genesis block, so the descriptors/keys creation time don't interfere with the blocks scanning process. + SetMockTime(test_setup->m_node.chainman->GetParams().GenesisBlock().nTime); CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()}; { LOCK(wallet.cs_wallet); diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index ce13a7701743..3d494871cf30 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -53,15 +53,14 @@ struct AddressTableEntryLessThan }; /* Determine address type from address purpose */ -static AddressTableEntry::Type translateTransactionType(wallet::AddressPurpose purpose, bool isMine) +constexpr AddressTableEntry::Type translateTransactionType(wallet::AddressPurpose purpose, bool isMine) { // "refund" addresses aren't shown, and change addresses aren't returned by getAddresses at all. switch (purpose) { case wallet::AddressPurpose::SEND: return AddressTableEntry::Sending; case wallet::AddressPurpose::RECEIVE: return AddressTableEntry::Receiving; case wallet::AddressPurpose::REFUND: return AddressTableEntry::Hidden; - // No default case to allow for compiler to warn - } + } // no default case, so the compiler can warn about missing cases assert(false); } diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 1bef88730638..7fe8d09cb306 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -121,6 +121,12 @@ static const CRPCConvertParam vRPCConvertParams[] = { "listunspent", 2, "addresses" }, { "listunspent", 3, "include_unsafe" }, { "listunspent", 4, "query_options" }, + { "listunspent", 4, "minimumAmount" }, + { "listunspent", 4, "maximumAmount" }, + { "listunspent", 4, "maximumCount" }, + { "listunspent", 4, "minimumSumAmount" }, + { "listunspent", 4, "include_immature_coinbase" }, + { "listunspent", 4, "coinType" }, { "getblock", 1, "verbosity" }, { "getblock", 1, "verbose" }, { "getblockheader", 1, "verbose" }, @@ -152,10 +158,37 @@ static const CRPCConvertParam vRPCConvertParams[] = { "submitpackage", 0, "package" }, { "combinerawtransaction", 0, "txs" }, { "fundrawtransaction", 1, "options" }, + { "fundrawtransaction", 1, "add_inputs"}, + { "fundrawtransaction", 1, "include_unsafe"}, + { "fundrawtransaction", 1, "minconf"}, + { "fundrawtransaction", 1, "maxconf"}, + { "fundrawtransaction", 1, "changePosition"}, + { "fundrawtransaction", 1, "includeWatching"}, + { "fundrawtransaction", 1, "lockUnspents"}, + { "fundrawtransaction", 1, "fee_rate"}, + { "fundrawtransaction", 1, "feeRate"}, + { "fundrawtransaction", 1, "subtractFeeFromOutputs"}, + { "fundrawtransaction", 1, "input_sizes"}, + { "fundrawtransaction", 1, "conf_target"}, + { "fundrawtransaction", 1, "solving_data"}, + { "fundrawtransaction", 1, "use_cj"}, { "walletcreatefundedpsbt", 0, "inputs" }, { "walletcreatefundedpsbt", 1, "outputs" }, { "walletcreatefundedpsbt", 2, "locktime" }, { "walletcreatefundedpsbt", 3, "options" }, + { "walletcreatefundedpsbt", 3, "add_inputs"}, + { "walletcreatefundedpsbt", 3, "include_unsafe"}, + { "walletcreatefundedpsbt", 3, "minconf"}, + { "walletcreatefundedpsbt", 3, "maxconf"}, + { "walletcreatefundedpsbt", 3, "changePosition"}, + { "walletcreatefundedpsbt", 3, "includeWatching"}, + { "walletcreatefundedpsbt", 3, "lockUnspents"}, + { "walletcreatefundedpsbt", 3, "fee_rate"}, + { "walletcreatefundedpsbt", 3, "feeRate"}, + { "walletcreatefundedpsbt", 3, "subtractFeeFromOutputs"}, + { "walletcreatefundedpsbt", 3, "conf_target"}, + { "walletcreatefundedpsbt", 3, "solving_data"}, + { "walletcreatefundedpsbt", 3, "use_cj"}, { "walletcreatefundedpsbt", 4, "bip32derivs" }, { "walletprocesspsbt", 1, "sign" }, { "walletprocesspsbt", 3, "bip32derivs" }, @@ -182,12 +215,42 @@ static const CRPCConvertParam vRPCConvertParams[] = { "send", 1, "conf_target" }, { "send", 3, "fee_rate"}, { "send", 4, "options" }, + { "send", 4, "add_inputs"}, + { "send", 4, "include_unsafe"}, + { "send", 4, "minconf"}, + { "send", 4, "maxconf"}, + { "send", 4, "add_to_wallet"}, + { "send", 4, "change_position"}, + { "send", 4, "fee_rate"}, + { "send", 4, "include_watching"}, + { "send", 4, "inputs"}, + { "send", 4, "locktime"}, + { "send", 4, "lock_unspents"}, + { "send", 4, "psbt"}, + { "send", 4, "subtract_fee_from_outputs"}, + { "send", 4, "conf_target"}, + { "send", 4, "solving_data"}, + { "send", 4, "use_cj"}, { "sendall", 0, "recipients" }, { "sendall", 1, "conf_target" }, { "sendall", 3, "fee_rate"}, { "sendall", 4, "options" }, + { "sendall", 4, "add_to_wallet"}, + { "sendall", 4, "fee_rate"}, + { "sendall", 4, "include_watching"}, + { "sendall", 4, "inputs"}, + { "sendall", 4, "locktime"}, + { "sendall", 4, "lock_unspents"}, + { "sendall", 4, "psbt"}, + { "sendall", 4, "send_max"}, + { "sendall", 4, "minconf"}, + { "sendall", 4, "maxconf"}, + { "sendall", 4, "conf_target"}, + { "sendall", 4, "solving_data"}, + { "sendall", 4, "use_cj"}, { "simulaterawtransaction", 0, "rawtxs" }, { "simulaterawtransaction", 1, "options" }, + { "simulaterawtransaction", 1, "include_watchonly"}, { "importprivkey", 2, "rescan" }, { "importelectrumwallet", 1, "index" }, { "importaddress", 2, "rescan" }, @@ -195,6 +258,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "importpubkey", 2, "rescan" }, { "importmulti", 0, "requests" }, { "importmulti", 1, "options" }, + { "importmulti", 1, "rescan" }, { "importdescriptors", 0, "requests" }, { "listdescriptors", 0, "private" }, { "verifychain", 0, "checklevel" }, diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 440ac1a33df8..5dfe27db2756 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -409,7 +409,7 @@ std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq) * Process named arguments into a vector of positional arguments, based on the * passed-in specification for the RPC call's arguments. */ -static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, const std::vector& argNames) +static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, const std::vector>& argNames) { JSONRPCRequest out = in; out.params = UniValue(UniValue::VARR); @@ -434,7 +434,9 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c // "args" parameter, if present. int hole = 0; int initial_hole_size = 0; - for (const std::string &argNamePattern: argNames) { + const std::string* initial_param = nullptr; + UniValue options{UniValue::VOBJ}; + for (const auto& [argNamePattern, named_only]: argNames) { std::vector vargNames = SplitString(argNamePattern, '|'); auto fr = argsIn.end(); for (const std::string & argName : vargNames) { @@ -443,7 +445,22 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c break; } } - if (fr != argsIn.end()) { + + // Handle named-only parameters by pushing them into a temporary options + // object, and then pushing the accumulated options as the next + // positional argument. + if (named_only) { + if (fr != argsIn.end()) { + if (options.exists(fr->first)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + fr->first + " specified multiple times"); + } + options.pushKVEnd(fr->first, *fr->second); + argsIn.erase(fr); + } + continue; + } + + if (!options.empty() || fr != argsIn.end()) { for (int i = 0; i < hole; ++i) { // Fill hole between specified parameters with JSON nulls, // but not at the end (for backwards compatibility with calls @@ -451,12 +468,26 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c out.params.push_back(UniValue()); } hole = 0; - out.params.push_back(*fr->second); - argsIn.erase(fr); + if (!initial_param) initial_param = &argNamePattern; } else { hole += 1; if (out.params.empty()) initial_hole_size = hole; } + + // If named input parameter "fr" is present, push it onto out.params. If + // options are present, push them onto out.params. If both are present, + // throw an error. + if (fr != argsIn.end()) { + if (!options.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + fr->first + " conflicts with parameter " + options.getKeys().front()); + } + out.params.push_back(*fr->second); + argsIn.erase(fr); + } + if (!options.empty()) { + out.params.push_back(std::move(options)); + options = UniValue{UniValue::VOBJ}; + } } // If leftover "args" param was found, use it as a source of positional // arguments and add named arguments after. This is a convenience for @@ -464,9 +495,8 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c // arguments as described in doc/JSON-RPC-interface.md#parameter-passing auto positional_args{argsIn.extract("args")}; if (positional_args && positional_args.mapped()->isArray()) { - const bool has_named_arguments{initial_hole_size < (int)argNames.size()}; - if (initial_hole_size < (int)positional_args.mapped()->size() && has_named_arguments) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + argNames[initial_hole_size] + " specified twice both as positional and named argument"); + if (initial_hole_size < (int)positional_args.mapped()->size() && initial_param) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + *initial_param + " specified twice both as positional and named argument"); } // Assign positional_args to out.params and append named_args after. UniValue named_args{std::move(out.params)}; diff --git a/src/rpc/server.h b/src/rpc/server.h index 73b362d50d89..b8348e4aa663 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -93,7 +93,7 @@ class CRPCCommand using Actor = std::function; //! Constructor taking Actor callback supporting multiple handlers. - CRPCCommand(std::string category, std::string name, Actor actor, std::vector args, intptr_t unique_id) + CRPCCommand(std::string category, std::string name, Actor actor, std::vector> args, intptr_t unique_id) : category(std::move(category)), name(std::move(name)), actor(std::move(actor)), argNames(std::move(args)), unique_id(unique_id) { @@ -113,7 +113,16 @@ class CRPCCommand std::string category; std::string name; Actor actor; - std::vector argNames; + //! List of method arguments and whether they are named-only. Incoming RPC + //! requests contain a "params" field that can either be an array containing + //! unnamed arguments or an object containing named arguments. The + //! "argNames" vector is used in the latter case to transform the params + //! object into an array. Each argument in "argNames" gets mapped to a + //! unique position in the array, based on the order it is listed, unless + //! the argument is a named-only argument with argNames[x].second set to + //! true. Named-only arguments are combined into a JSON object that is + //! appended after other arguments, see transformNamedArguments for details. + std::vector> argNames; intptr_t unique_id; }; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 362ba3001e73..8bb5e2ff4967 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -367,7 +367,8 @@ struct Sections { case RPCArg::Type::NUM: case RPCArg::Type::AMOUNT: case RPCArg::Type::RANGE: - case RPCArg::Type::BOOL: { + case RPCArg::Type::BOOL: + case RPCArg::Type::OBJ_NAMED_PARAMS: { if (is_top_level_arg) return; // Nothing more to do for non-recursive types on first recursion auto left = indent; if (arg.m_opts.type_str.size() != 0 && push_name) { @@ -463,12 +464,32 @@ RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector named_args; + // Map of parameter names and types just used to check whether the names are + // unique. Parameter names always need to be unique, with the exception that + // there can be pairs of POSITIONAL and NAMED parameters with the same name. + enum ParamType { POSITIONAL = 1, NAMED = 2, NAMED_ONLY = 4 }; + std::map param_names; + for (const auto& arg : m_args) { std::vector names = SplitString(arg.m_names, '|'); // Should have unique named arguments for (const std::string& name : names) { - CHECK_NONFATAL(named_args.insert(name).second); + auto& param_type = param_names[name]; + CHECK_NONFATAL(!(param_type & POSITIONAL)); + CHECK_NONFATAL(!(param_type & NAMED_ONLY)); + param_type |= POSITIONAL; + } + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& inner : arg.m_inner) { + std::vector inner_names = SplitString(inner.m_names, '|'); + for (const std::string& inner_name : inner_names) { + auto& param_type = param_names[inner_name]; + CHECK_NONFATAL(!(param_type & POSITIONAL) || inner.m_opts.also_positional); + CHECK_NONFATAL(!(param_type & NAMED)); + CHECK_NONFATAL(!(param_type & NAMED_ONLY)); + param_type |= inner.m_opts.also_positional ? NAMED : NAMED_ONLY; + } + } } // Default value type should match argument type only when defined if (arg.m_fallback.index() == 2) { @@ -631,12 +652,17 @@ bool RPCHelpMan::IsValidNumArgs(size_t num_args) const return num_required_args <= num_args && num_args <= m_args.size(); } -std::vector RPCHelpMan::GetArgNames() const +std::vector> RPCHelpMan::GetArgNames() const { - std::vector ret; + std::vector> ret; ret.reserve(m_args.size()); for (const auto& arg : m_args) { - ret.emplace_back(arg.m_names); + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& inner : arg.m_inner) { + ret.emplace_back(inner.m_names, /*named_only=*/true); + } + } + ret.emplace_back(arg.m_names, /*named_only=*/false); } return ret; } @@ -668,20 +694,31 @@ std::string RPCHelpMan::ToString() const // Arguments Sections sections; + Sections named_only_sections; for (size_t i{0}; i < m_args.size(); ++i) { const auto& arg = m_args.at(i); if (arg.m_opts.hidden) break; // Any arg that follows is also hidden - if (i == 0) ret += "\nArguments:\n"; - // Push named argument name and description sections.m_sections.emplace_back(::ToString(i + 1) + ". " + arg.GetFirstName(), arg.ToDescriptionString(/*is_named_arg=*/true)); sections.m_max_pad = std::max(sections.m_max_pad, sections.m_sections.back().m_left.size()); // Recursively push nested args sections.Push(arg); + + // Push named-only argument sections + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& arg_inner : arg.m_inner) { + named_only_sections.PushSection({arg_inner.GetFirstName(), arg_inner.ToDescriptionString(/*is_named_arg=*/true)}); + named_only_sections.Push(arg_inner); + } + } } + + if (!sections.m_sections.empty()) ret += "\nArguments:\n"; ret += sections.ToString(); + if (!named_only_sections.m_sections.empty()) ret += "\nNamed Arguments:\n"; + ret += named_only_sections.ToString(); // Result ret += m_results.ToDescriptionString(); @@ -695,17 +732,30 @@ std::string RPCHelpMan::ToString() const UniValue RPCHelpMan::GetArgMap() const { UniValue arr{UniValue::VARR}; + + auto push_back_arg_info = [&arr](const std::string& rpc_name, int pos, const std::string& arg_name, const RPCArg::Type& type) { + UniValue map{UniValue::VARR}; + map.push_back(rpc_name); + map.push_back(pos); + map.push_back(arg_name); + map.push_back(type == RPCArg::Type::STR || + type == RPCArg::Type::STR_HEX); + arr.push_back(map); + }; + for (int i{0}; i < int(m_args.size()); ++i) { const auto& arg = m_args.at(i); std::vector arg_names = SplitString(arg.m_names, '|'); for (const auto& arg_name : arg_names) { - UniValue map{UniValue::VARR}; - map.push_back(m_name); - map.push_back(i); - map.push_back(arg_name); - map.push_back(arg.m_type == RPCArg::Type::STR || - arg.m_type == RPCArg::Type::STR_HEX); - arr.push_back(map); + push_back_arg_info(m_name, i, arg_name, arg.m_type); + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& inner : arg.m_inner) { + std::vector inner_names = SplitString(inner.m_names, '|'); + for (const std::string& inner_name : inner_names) { + push_back_arg_info(m_name, i, inner_name, inner.m_type); + } + } + } } } return arr; @@ -734,6 +784,7 @@ static std::optional ExpectedType(RPCArg::Type type) return UniValue::VBOOL; } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { return UniValue::VOBJ; } @@ -807,6 +858,7 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const break; } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { ret += "json object"; break; @@ -835,6 +887,7 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const } // no default case, so the compiler can warn about missing cases } ret += ")"; + if (m_type == Type::OBJ_NAMED_PARAMS) ret += " Options object that can be used to pass named arguments, listed below."; ret += m_description.empty() ? "" : " " + m_description; return ret; } @@ -1081,6 +1134,7 @@ std::string RPCArg::ToStringObj(const bool oneline) const } return res + "...]"; case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: // Currently unused, so avoid writing dead code NONFATAL_UNREACHABLE(); @@ -1113,6 +1167,7 @@ std::string RPCArg::ToString(const bool oneline) const return GetFirstName(); } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { const std::string res = Join(m_inner, ",", [&](const RPCArg& i) { return i.ToStringObj(oneline); }); if (m_type == Type::OBJ) { diff --git a/src/rpc/util.h b/src/rpc/util.h index e97f935fa9bb..34629ca5d608 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -151,6 +151,15 @@ struct RPCArgOptions { std::string oneline_description{}; //!< Should be empty unless it is supposed to override the auto-generated summary line std::vector type_str{}; //!< Should be empty unless it is supposed to override the auto-generated type strings. Vector length is either 0 or 2, m_opts.type_str.at(0) will override the type of the value in a key-value pair, m_opts.type_str.at(1) will override the type in the argument description. bool hidden{false}; //!< For testing only + bool also_positional{false}; //!< If set allows a named-parameter field in an OBJ_NAMED_PARAM options object + //!< to have the same name as a top-level parameter. By default the RPC + //!< framework disallows this, because if an RPC request passes the value by + //!< name, it is assigned to top-level parameter position, not to the options + //!< position, defeating the purpose of using OBJ_NAMED_PARAMS instead OBJ for + //!< that option. But sometimes it makes sense to allow less-commonly used + //!< options to be passed by name only, and more commonly used options to be + //!< passed by name or position, so the RPC framework allows this as long as + //!< methods set the also_positional flag and read values from both positions. }; struct RPCArg { @@ -160,6 +169,13 @@ struct RPCArg { STR, NUM, BOOL, + OBJ_NAMED_PARAMS, //!< Special type that behaves almost exactly like + //!< OBJ, defining an options object with a list of + //!< pre-defined keys. The only difference between OBJ + //!< and OBJ_NAMED_PARAMS is that OBJ_NAMED_PARMS + //!< also allows the keys to be passed as top-level + //!< named parameters, as a more convenient way to pass + //!< options to the RPC method without nesting them. OBJ_USER_KEYS, //!< Special type where the user must set the keys e.g. to define multiple addresses; as opposed to e.g. an options object where the keys are predefined AMOUNT, //!< Special type representing a floating point amount (can be either NUM or STR) STR_HEX, //!< Special type that is a STR with only hex chars @@ -204,7 +220,7 @@ struct RPCArg { m_description{std::move(description)}, m_opts{std::move(opts)} { - CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_USER_KEYS); + CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_NAMED_PARAMS && type != Type::OBJ_USER_KEYS); } RPCArg( @@ -221,7 +237,7 @@ struct RPCArg { m_description{std::move(description)}, m_opts{std::move(opts)} { - CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ || type == Type::OBJ_USER_KEYS); + CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ || type == Type::OBJ_NAMED_PARAMS || type == Type::OBJ_USER_KEYS); } bool IsOptional() const; @@ -428,8 +444,8 @@ class RPCHelpMan UniValue GetArgMap() const; /** If the supplied number of args is neither too small nor too high */ bool IsValidNumArgs(size_t num_args) const; - - std::vector GetArgNames() const; + //! Return list of arguments and whether they are named-only. + std::vector> GetArgNames() const; const std::string m_name; diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index df0146e63a87..1728ef9b55dd 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -997,7 +997,7 @@ void FuncDIP3Protx(TestChainSetup& setup) // check MN reward payments for (size_t i = 0; i < 20; i++) { auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(tip_index()); - BOOST_ASSERT(dmnExpectedPayee); + BOOST_REQUIRE(dmnExpectedPayee); CBlock block = setup.CreateAndProcessBlock({}, coinbase_pk); sync_dmn_tip(); @@ -1108,7 +1108,7 @@ void FuncDIP3Protx(TestChainSetup& setup) bool foundRevived = false; for (size_t i = 0; i < 20; i++) { auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(tip_index()); - BOOST_ASSERT(dmnExpectedPayee); + BOOST_REQUIRE(dmnExpectedPayee); if (dmnExpectedPayee->proTxHash == dmnHashes[0]) { foundRevived = true; } diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 874eb8a11bcf..e943c14c42ed 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -73,6 +73,7 @@ const std::vector RPC_COMMANDS_NOT_SAFE_FOR_FUZZING{ "addpeeraddress", // avoid DNS lookups "dumptxoutset", // avoid writing to disk "dumpwallet", // avoid writing to disk + "enumeratesigners", "echoipc", // avoid assertion failure (Assertion `"EnsureAnyNodeContext(request.context).init" && check' failed.) "generatetoaddress", // avoid prohibitively slow execution (when `num_blocks` is large) "generatetodescriptor", // avoid prohibitively slow execution (when `nblocks` is large) diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 0501a4f264fb..835bd1204caf 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -44,11 +44,11 @@ class RPCTestingSetup : public TestingSetup RPCTestingSetup() : TestingSetup(CBaseChainParams::MAIN, {"-deprecatedrpc=legacy_mn"}) {} - UniValue TransformParams(const UniValue& params, std::vector arg_names); + UniValue TransformParams(const UniValue& params, std::vector> arg_names); UniValue CallRPC(std::string args); }; -UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector arg_names) +UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector> arg_names) { UniValue transformed_params; CRPCTable table; @@ -89,7 +89,7 @@ BOOST_FIXTURE_TEST_SUITE(rpc_tests, RPCTestingSetup) BOOST_AUTO_TEST_CASE(rpc_namedparams) { - const std::vector arg_names{"arg1", "arg2", "arg3", "arg4", "arg5"}; + const std::vector> arg_names{{"arg1", false}, {"arg2", false}, {"arg3", false}, {"arg4", false}, {"arg5", false}}; // Make sure named arguments are transformed into positional arguments in correct places separated by nulls BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg2": 2, "arg4": 4})"), arg_names).write(), "[null,2,null,4]"); @@ -114,6 +114,28 @@ BOOST_AUTO_TEST_CASE(rpc_namedparams) BOOST_CHECK_EQUAL(TransformParams(JSON(R"([1,2,3,4,5,6,7,8,9,10])"), arg_names).write(), "[1,2,3,4,5,6,7,8,9,10]"); } +BOOST_AUTO_TEST_CASE(rpc_namedonlyparams) +{ + const std::vector> arg_names{{"arg1", false}, {"arg2", false}, {"opt1", true}, {"opt2", true}, {"options", false}}; + + // Make sure optional parameters are really optional. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2})"), arg_names).write(), "[1,2]"); + + // Make sure named-only parameters are passed as options. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "opt1": 10, "opt2": 20})"), arg_names).write(), R"([1,2,{"opt1":10,"opt2":20}])"); + + // Make sure options can be passed directly. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "options":{"opt1": 10, "opt2": 20}})"), arg_names).write(), R"([1,2,{"opt1":10,"opt2":20}])"); + + // Make sure options and named parameters conflict. + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "opt1": 10, "options":{"opt1": 10}})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Parameter options conflicts with parameter opt1"})")); + + // Make sure options object specified through args array conflicts. + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"args": [1, 2, {"opt1": 10}], "opt2": 20})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Parameter options specified twice both as positional and named argument"})")); +} + BOOST_AUTO_TEST_CASE(rpc_rawparams) { // Test raw transaction API argument handling @@ -487,6 +509,53 @@ BOOST_AUTO_TEST_CASE(rpc_getblockstats_calculate_percentiles_by_size) } } +// Make sure errors are triggered appropriately if parameters have the same names. +BOOST_AUTO_TEST_CASE(check_dup_param_names) +{ + enum ParamType { POSITIONAL, NAMED, NAMED_ONLY }; + auto make_rpc = [](std::vector> param_names) { + std::vector params; + std::vector options; + auto push_options = [&] { if (!options.empty()) params.emplace_back(RPCArg{strprintf("options%i", params.size()), RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", std::move(options)}); }; + for (auto& [param_name, param_type] : param_names) { + if (param_type == POSITIONAL) { + push_options(); + params.emplace_back(std::move(param_name), RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "description"); + } else { + options.emplace_back(std::move(param_name), RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "description", RPCArgOptions{.also_positional = param_type == NAMED}); + } + } + push_options(); + return RPCHelpMan{"method_name", "description", params, RPCResults{}, RPCExamples{""}}; + }; + + // No errors if parameter names are unique. + make_rpc({{"p1", POSITIONAL}, {"p2", POSITIONAL}}); + make_rpc({{"p1", POSITIONAL}, {"p2", NAMED}}); + make_rpc({{"p1", POSITIONAL}, {"p2", NAMED_ONLY}}); + make_rpc({{"p1", NAMED}, {"p2", POSITIONAL}}); + make_rpc({{"p1", NAMED}, {"p2", NAMED}}); + make_rpc({{"p1", NAMED}, {"p2", NAMED_ONLY}}); + make_rpc({{"p1", NAMED_ONLY}, {"p2", POSITIONAL}}); + make_rpc({{"p1", NAMED_ONLY}, {"p2", NAMED}}); + make_rpc({{"p1", NAMED_ONLY}, {"p2", NAMED_ONLY}}); + + // Error if parameters names are duplicates, unless one parameter is + // positional and the other is named and .also_positional is true. + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", POSITIONAL}}), NonFatalCheckError); + make_rpc({{"p1", POSITIONAL}, {"p1", NAMED}}); + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + make_rpc({{"p1", NAMED}, {"p1", POSITIONAL}}); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", POSITIONAL}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + + // Make sure duplicate aliases are detected too. + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p2|p1", NAMED_ONLY}}), NonFatalCheckError); +} + BOOST_AUTO_TEST_CASE(help_example) { // test different argument types diff --git a/src/txrequest.cpp b/src/txrequest.cpp index ae1beed5c634..5bb4c07f3ec5 100644 --- a/src/txrequest.cpp +++ b/src/txrequest.cpp @@ -9,8 +9,12 @@ #include #include -#include +#include #include +#include +#include +#include +#include #include #include @@ -66,7 +70,7 @@ struct Announcement { const bool m_preferred : 1; /** What state this announcement is in. - * This is a uint8_t instead of a State to silence a GCC warning in versions prior to 8.4 and 9.3. + * This is a uint8_t instead of a State to silence a GCC warning in versions prior to 9.3. * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61414 */ uint8_t m_state : 3 {static_cast(State::CANDIDATE_DELAYED)}; diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 20bcc4f13088..ef4651152931 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -757,7 +757,8 @@ void BerkeleyDatabase::ReloadDbEnv() env->ReloadDbEnv(); } -BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch) +BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch, Span prefix) + : m_key_prefix(prefix.begin(), prefix.end()) { if (!database.m_db.get()) { throw std::runtime_error(STR_INTERNAL_BUG("BerkeleyDatabase does not exist")); @@ -774,19 +775,30 @@ DatabaseCursor::Status BerkeleyCursor::Next(DataStream& ssKey, DataStream& ssVal { if (m_cursor == nullptr) return Status::FAIL; // Read at cursor - SafeDbt datKey; + SafeDbt datKey(m_key_prefix.data(), m_key_prefix.size()); SafeDbt datValue; - int ret = m_cursor->get(datKey, datValue, DB_NEXT); + int ret = -1; + if (m_first && !m_key_prefix.empty()) { + ret = m_cursor->get(datKey, datValue, DB_SET_RANGE); + } else { + ret = m_cursor->get(datKey, datValue, DB_NEXT); + } + m_first = false; if (ret == DB_NOTFOUND) { return Status::DONE; } - if (ret != 0 || datKey.get_data() == nullptr || datValue.get_data() == nullptr) { + if (ret != 0) { return Status::FAIL; } + Span raw_key = SpanFromDbt(datKey); + if (!m_key_prefix.empty() && std::mismatch(raw_key.begin(), raw_key.end(), m_key_prefix.begin(), m_key_prefix.end()).second != m_key_prefix.end()) { + return Status::DONE; + } + // Convert to streams ssKey.clear(); - ssKey.write(SpanFromDbt(datKey)); + ssKey.write(raw_key); ssValue.clear(); ssValue.write(SpanFromDbt(datValue)); return Status::MORE; @@ -805,6 +817,12 @@ std::unique_ptr BerkeleyBatch::GetNewCursor() return std::make_unique(m_database, *this); } +std::unique_ptr BerkeleyBatch::GetNewPrefixCursor(Span prefix) +{ + if (!pdb) return nullptr; + return std::make_unique(m_database, *this, prefix); +} + bool BerkeleyBatch::TxnBegin() { if (!pdb || activeTxn) diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 2d9bf4ef0e33..7d287f6f25d5 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -159,9 +159,13 @@ class BerkeleyCursor : public DatabaseCursor { private: Dbc* m_cursor; + std::vector m_key_prefix; + bool m_first{true}; public: - explicit BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch); + // Constructor for cursor for records matching the prefix + // To match all records, an empty prefix may be provided. + explicit BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch, Span prefix = {}); ~BerkeleyCursor() override; Status Next(DataStream& key, DataStream& value) override; @@ -197,6 +201,7 @@ class BerkeleyBatch : public DatabaseBatch void Close() override; std::unique_ptr GetNewCursor() override; + std::unique_ptr GetNewPrefixCursor(Span prefix) override; bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 64c90612167b..a11a3217091d 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -207,7 +207,7 @@ class MinOutputGroupComparator } }; -util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng, +util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng, int max_weight, bool only_fully_mixed) { if (utxo_pool.empty()) return util::Error(); @@ -220,7 +220,7 @@ util::Result SelectCoinsSRD(const std::vector& utx // generated one, since SRD will result in a random change amount anyway; avoid making the // target needlessly large. if (!only_fully_mixed) { - target_value += CHANGE_LOWER; + target_value += CHANGE_LOWER + change_fee; } std::vector indexes; diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index de7c15496000..b874af9fa8fe 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -432,7 +432,7 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool * @param[in] max_weight The maximum allowed weight for a selection result to be valid * @returns If successful, a valid SelectionResult, otherwise, util::Error */ -util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng, +util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng, int max_weight, bool only_fully_mixed); // Original coin selection algorithm as a fallback diff --git a/src/wallet/db.h b/src/wallet/db.h index 55558ab87efe..1921b104db12 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -112,6 +112,7 @@ class DatabaseBatch virtual bool ErasePrefix(Span prefix) = 0; virtual std::unique_ptr GetNewCursor() = 0; + virtual std::unique_ptr GetNewPrefixCursor(Span prefix) = 0; virtual bool TxnBegin() = 0; virtual bool TxnCommit() = 0; virtual bool TxnAbort() = 0; diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 9e72c0dc3309..acd32c39f8df 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1526,7 +1526,7 @@ RPCHelpMan importmulti() }, }, RPCArgOptions{.oneline_description="requests"}}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."}, }, diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index cc03c59c79d3..3c09bab10cb1 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -560,7 +560,7 @@ RPCHelpMan listunspent() }, {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include outputs that are not safe to spend\n" "See description of \"safe\" attribute below."}, - {"query_options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "JSON with query options", + {"query_options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { {"minimumAmount", RPCArg::Type::AMOUNT, RPCArg::Default{0}, "Minimum value of each UTXO in " + CURRENCY_UNIT + ""}, {"maximumAmount", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"unlimited"}, "Maximum value of each UTXO in " + CURRENCY_UNIT + ""}, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index a39fa39e06ae..fa21a43c9660 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -477,9 +477,9 @@ RPCHelpMan settxfee() static std::vector FundTxDoc(bool solving_data = true) { std::vector args = { - {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, + {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks", RPCArgOptions{.also_positional = true}}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" - "\"" + FeeModes("\"\n\"") + "\""}, + "\"" + FeeModes("\"\n\"") + "\"", RPCArgOptions{.also_positional = true}}, }; if (solving_data) { args.push_back({"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" @@ -767,7 +767,7 @@ RPCHelpMan fundrawtransaction() "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "For backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", Cat>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{true}, "For a transaction with existing inputs, automatically include more if they are not enough."}, @@ -985,7 +985,7 @@ RPCHelpMan send() {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", Cat>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"},"Automatically include coins from the wallet to cover the target amount.\n"}, @@ -997,7 +997,7 @@ RPCHelpMan send() {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns a serialized transaction which will not be added to the wallet or broadcast"}, {"change_address", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The Dash address to receive the change"}, {"change_position", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, - {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, + {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/B.", RPCArgOptions{.also_positional = true}}, {"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch only.\n" "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, @@ -1098,11 +1098,11 @@ RPCHelpMan sendall() "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, { - "options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + "options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", Cat>( { {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns the serialized transaction without broadcasting or adding it to the wallet"}, - {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, + {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/B.", RPCArgOptions{.also_positional = true}}, {"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch-only.\n" "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, @@ -1447,7 +1447,7 @@ RPCHelpMan walletcreatefundedpsbt() OutputsDoc(), RPCArgOptions{.skip_type_check = true}}, {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", Cat>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"}, "Automatically include coins from the wallet to cover the target amount.\n"}, diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index e0a85beb6d8a..52f527f24a9e 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -1028,7 +1028,7 @@ RPCHelpMan simulaterawtransaction() {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, }, }, - {"options", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "Options", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see RPC importaddress)"}, }, diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index 804813097a9d..3c704c27c512 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -45,6 +45,7 @@ class DummyBatch : public DatabaseBatch void Close() override {} std::unique_ptr GetNewCursor() override { return std::make_unique(); } + std::unique_ptr GetNewPrefixCursor(Span prefix) override { return GetNewCursor(); } bool TxnBegin() override { return true; } bool TxnCommit() override { return true; } bool TxnAbort() override { return true; } diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index bb529e32fe3f..38fad606f74c 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -600,7 +600,7 @@ util::Result ChooseSelectionResult(const CWallet& wallet, const results.push_back(*knapsack_result); } else append_error(knapsack_result); - if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.rng_fast, max_inputs_weight, + if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_inputs_weight, coin_selection_params.m_coin_type == CoinType::ONLY_FULLY_MIXED)}) { srd_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*srd_result); diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 84ab061e4138..8d8510511f45 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -64,7 +65,11 @@ static bool BindBlobToStatement(sqlite3_stmt* stmt, Span blob, const std::string& description) { - int res = sqlite3_bind_blob(stmt, index, blob.data(), blob.size(), SQLITE_STATIC); + // Pass a pointer to the empty string "" below instead of passing the + // blob.data() pointer if the blob.data() pointer is null. Passing a null + // data pointer to bind_blob would cause sqlite to bind the SQL NULL value + // instead of the empty blob value X'', which would mess up SQL comparisons. + int res = sqlite3_bind_blob(stmt, index, blob.data() ? static_cast(blob.data()) : "", blob.size(), SQLITE_STATIC); if (res != SQLITE_OK) { LogPrintf("Unable to bind %s to statement: %s\n", description, sqlite3_errstr(res)); sqlite3_clear_bindings(stmt); @@ -540,6 +545,7 @@ DatabaseCursor::Status SQLiteCursor::Next(DataStream& key, DataStream& value) SQLiteCursor::~SQLiteCursor() { + sqlite3_clear_bindings(m_cursor_stmt); sqlite3_reset(m_cursor_stmt); int res = sqlite3_finalize(m_cursor_stmt); if (res != SQLITE_OK) { @@ -563,6 +569,48 @@ std::unique_ptr SQLiteBatch::GetNewCursor() return cursor; } +std::unique_ptr SQLiteBatch::GetNewPrefixCursor(Span prefix) +{ + if (!m_database.m_db) return nullptr; + + // To get just the records we want, the SQL statement does a comparison of the binary data + // where the data must be greater than or equal to the prefix, and less than + // the prefix incremented by one (when interpreted as an integer) + std::vector start_range(prefix.begin(), prefix.end()); + std::vector end_range(prefix.begin(), prefix.end()); + auto it = end_range.rbegin(); + for (; it != end_range.rend(); ++it) { + if (*it == std::byte(std::numeric_limits::max())) { + *it = std::byte(0); + continue; + } + *it = std::byte(std::to_integer(*it) + 1); + break; + } + if (it == end_range.rend()) { + // If the prefix is all 0xff bytes, clear end_range as we won't need it + end_range.clear(); + } + + auto cursor = std::make_unique(start_range, end_range); + if (!cursor) return nullptr; + + const char* stmt_text = end_range.empty() ? "SELECT key, value FROM main WHERE key >= ?" : + "SELECT key, value FROM main WHERE key >= ? AND key < ?"; + int res = sqlite3_prepare_v2(m_database.m_db, stmt_text, -1, &cursor->m_cursor_stmt, nullptr); + if (res != SQLITE_OK) { + throw std::runtime_error(strprintf( + "SQLiteDatabase: Failed to setup cursor SQL statement: %s\n", sqlite3_errstr(res))); + } + + if (!BindBlobToStatement(cursor->m_cursor_stmt, 1, cursor->m_prefix_range_start, "prefix_start")) return nullptr; + if (!end_range.empty()) { + if (!BindBlobToStatement(cursor->m_cursor_stmt, 2, cursor->m_prefix_range_end, "prefix_end")) return nullptr; + } + + return cursor; +} + bool SQLiteBatch::TxnBegin() { if (!m_database.m_db || sqlite3_get_autocommit(m_database.m_db) == 0) return false; diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index b5d142434b59..5c379870bbc9 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -16,12 +16,21 @@ struct sqlite3; namespace wallet { class SQLiteDatabase; +/** RAII class that provides a database cursor */ class SQLiteCursor : public DatabaseCursor { public: sqlite3_stmt* m_cursor_stmt{nullptr}; + // Copies of the prefix things for the prefix cursor. + // Prevents SQLite from accessing temp variables for the prefix things. + std::vector m_prefix_range_start; + std::vector m_prefix_range_end; explicit SQLiteCursor() {} + explicit SQLiteCursor(std::vector start_range, std::vector end_range) + : m_prefix_range_start(std::move(start_range)), + m_prefix_range_end(std::move(end_range)) + {} ~SQLiteCursor() override; Status Next(DataStream& key, DataStream& value) override; @@ -64,6 +73,7 @@ class SQLiteBatch : public DatabaseBatch void Close() override; std::unique_ptr GetNewCursor() override; + std::unique_ptr GetNewPrefixCursor(Span prefix) override; bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 5d521c364eff..da7bc9bcecca 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -1048,7 +1048,7 @@ static util::Result SelectCoinsSRDResult(const CAmount& target, CoinEligibilityFilter filter(0, 0, 0); Groups group = GroupOutputs(*wallet, coin_setup(*wallet), cs_params, {{filter}})[filter].all_groups; - return wallet::SelectCoinsSRD(group.positive_group, target, cs_params.rng_fast, max_weight, false); + return wallet::SelectCoinsSRD(group.positive_group, target, cs_params.m_change_fee, cs_params.rng_fast, max_weight, false); } BOOST_AUTO_TEST_CASE(srd_tests) diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index 8aa8bc2d43cd..47c160fd64d5 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -9,14 +9,58 @@ #include #include +#include #include +#include +#ifdef USE_BDB #include +#endif +#ifdef USE_SQLITE +#include +#endif +#include +#include // for WALLET_FLAG_DESCRIPTORS #include #include #include +inline std::ostream& operator<<(std::ostream& os, const std::pair& kv) +{ + Span key{kv.first}, value{kv.second}; + os << "(\"" << std::string_view{reinterpret_cast(key.data()), key.size()} << "\", \"" + << std::string_view{reinterpret_cast(key.data()), key.size()} << "\")"; + return os; +} + namespace wallet { + +static Span StringBytes(std::string_view str) +{ + return AsBytes({str.data(), str.size()}); +} + +static SerializeData StringData(std::string_view str) +{ + auto bytes = StringBytes(str); + return SerializeData{bytes.begin(), bytes.end()}; +} + +static void CheckPrefix(DatabaseBatch& batch, Span prefix, MockableData expected) +{ + std::unique_ptr cursor = batch.GetNewPrefixCursor(prefix); + MockableData actual; + while (true) { + DataStream key, value; + DatabaseCursor::Status status = cursor->Next(key, value); + if (status == DatabaseCursor::Status::DONE) break; + BOOST_CHECK(status == DatabaseCursor::Status::MORE); + BOOST_CHECK( + actual.emplace(SerializeData(key.begin(), key.end()), SerializeData(value.begin(), value.end())).second); + } + BOOST_CHECK_EQUAL_COLLECTIONS(actual.begin(), actual.end(), expected.begin(), expected.end()); +} + BOOST_FIXTURE_TEST_SUITE(db_tests, BasicTestingSetup) static std::shared_ptr GetWalletEnv(const fs::path& path, fs::path& database_filename) @@ -82,5 +126,88 @@ BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_free_instance) BOOST_CHECK(env_2_a == env_2_b); } +static std::vector> TestDatabases(const fs::path& path_root) +{ + std::vector> dbs; + DatabaseOptions options; + DatabaseStatus status; + bilingual_str error; +#ifdef USE_BDB + dbs.emplace_back(MakeBerkeleyDatabase(path_root / "bdb", options, status, error)); +#endif +#ifdef USE_SQLITE + dbs.emplace_back(MakeSQLiteDatabase(path_root / "sqlite", options, status, error)); +#endif + dbs.emplace_back(CreateMockableWalletDatabase()); + return dbs; +} + +BOOST_AUTO_TEST_CASE(db_cursor_prefix_range_test) +{ + // Test each supported db + for (const auto& database : TestDatabases(m_path_root)) { + std::vector prefixes = {"", "FIRST", "SECOND", "P\xfe\xff", "P\xff\x01", "\xff\xff"}; + + // Write elements to it + std::unique_ptr handler = Assert(database)->MakeBatch(); + for (unsigned int i = 0; i < 10; i++) { + for (const auto& prefix : prefixes) { + BOOST_CHECK(handler->Write(std::make_pair(prefix, i), i)); + } + } + + // Now read all the items by prefix and verify that each element gets parsed correctly + for (const auto& prefix : prefixes) { + DataStream s_prefix; + s_prefix << prefix; + std::unique_ptr cursor = handler->GetNewPrefixCursor(s_prefix); + DataStream key; + DataStream value; + for (int i = 0; i < 10; i++) { + DatabaseCursor::Status status = cursor->Next(key, value); + BOOST_CHECK_EQUAL(status, DatabaseCursor::Status::MORE); + + std::string key_back; + unsigned int i_back; + key >> key_back >> i_back; + BOOST_CHECK_EQUAL(key_back, prefix); + + unsigned int value_back; + value >> value_back; + BOOST_CHECK_EQUAL(value_back, i_back); + } + + // Let's now read it once more, it should return DONE + BOOST_CHECK(cursor->Next(key, value) == DatabaseCursor::Status::DONE); + } + } +} + +// Lower level DatabaseBase::GetNewPrefixCursor test, to cover cases that aren't +// covered in the higher level test above. The higher level test uses +// serialized strings which are prefixed with string length, so it doesn't test +// truly empty prefixes or prefixes that begin with \xff +BOOST_AUTO_TEST_CASE(db_cursor_prefix_byte_test) +{ + const MockableData::value_type + e{StringData(""), StringData("e")}, + p{StringData("prefix"), StringData("p")}, + ps{StringData("prefixsuffix"), StringData("ps")}, + f{StringData("\xff"), StringData("f")}, + fs{StringData("\xffsuffix"), StringData("fs")}, + ff{StringData("\xff\xff"), StringData("ff")}, + ffs{StringData("\xff\xffsuffix"), StringData("ffs")}; + for (const auto& database : TestDatabases(m_path_root)) { + std::unique_ptr batch = database->MakeBatch(); + for (const auto& [k, v] : {e, p, ps, f, fs, ff, ffs}) { + batch->Write(MakeUCharSpan(k), MakeUCharSpan(v)); + } + CheckPrefix(*batch, StringBytes(""), {e, p, ps, f, fs, ff, ffs}); + CheckPrefix(*batch, StringBytes("prefix"), {p, ps}); + CheckPrefix(*batch, StringBytes("\xff"), {f, fs, ff, ffs}); + CheckPrefix(*batch, StringBytes("\xff\xff"), {ff, ffs}); + } +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 4903b5477574..a3ba1d996d50 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -89,8 +89,11 @@ FUZZ_TARGET(coinselection) // Run coinselection algorithms const auto result_bnb = SelectCoinsBnB(group_pos, target, cost_of_change, MAX_STANDARD_TX_SIZE); - auto result_srd = SelectCoinsSRD(group_pos, target, fast_random_context, MAX_STANDARD_TX_SIZE, false); - if (result_srd) result_srd->ComputeAndSetWaste(cost_of_change, cost_of_change, 0); + auto result_srd = SelectCoinsSRD(group_pos, target, coin_params.m_change_fee, fast_random_context, MAX_STANDARD_TX_SIZE, false); + if (result_srd) { + assert(result_srd->GetChange(CHANGE_LOWER, coin_params.m_change_fee) > 0); // Demonstrate that SRD creates change of at least CHANGE_LOWER + result_srd->ComputeAndSetWaste(cost_of_change, cost_of_change, 0); + } CAmount change_target{GenerateChangeTarget(target, coin_params.m_change_fee, fast_random_context)}; auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context, diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index 869b9a2f763a..e2ccc68b0c0c 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -106,6 +106,17 @@ CTxDestination getNewDestination(CWallet& w, OutputType output_type) return *Assert(w.GetNewDestination("")); } +// BytePrefix compares equality with other byte spans that begin with the same prefix. +struct BytePrefix { Span prefix; }; +bool operator<(BytePrefix a, Span b) { return a.prefix < b.subspan(0, std::min(a.prefix.size(), b.size())); } +bool operator<(Span a, BytePrefix b) { return a.subspan(0, std::min(a.size(), b.prefix.size())) < b.prefix; } + +MockableCursor::MockableCursor(const MockableData& records, bool pass, Span prefix) +{ + m_pass = pass; + std::tie(m_cursor, m_cursor_end) = records.equal_range(BytePrefix{prefix}); +} + DatabaseCursor::Status MockableCursor::Next(DataStream& key, DataStream& value) { if (!m_pass) { @@ -114,6 +125,8 @@ DatabaseCursor::Status MockableCursor::Next(DataStream& key, DataStream& value) if (m_cursor == m_cursor_end) { return Status::DONE; } + key.clear(); + value.clear(); const auto& [key_data, value_data] = *m_cursor; key.write(key_data); value.write(value_data); @@ -131,6 +144,7 @@ bool MockableBatch::ReadKey(DataStream&& key, DataStream& value) if (it == m_records.end()) { return false; } + value.clear(); value.write(it->second); return true; } @@ -186,7 +200,7 @@ bool MockableBatch::ErasePrefix(Span prefix) return true; } -std::unique_ptr CreateMockableWalletDatabase(std::map records) +std::unique_ptr CreateMockableWalletDatabase(MockableData records) { return std::make_unique(records); } diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index fa58f061f785..ffa4e561e697 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -52,14 +52,17 @@ std::string getnewaddress(wallet::CWallet& w); /** Returns a new destination, of an specific type, from the wallet */ CTxDestination getNewDestination(wallet::CWallet& w, OutputType output_type); +using MockableData = std::map>; + class MockableCursor: public DatabaseCursor { public: - std::map::const_iterator m_cursor; - std::map::const_iterator m_cursor_end; + MockableData::const_iterator m_cursor; + MockableData::const_iterator m_cursor_end; bool m_pass; - explicit MockableCursor(const std::map& records, bool pass) : m_cursor(records.begin()), m_cursor_end(records.end()), m_pass(pass) {} + explicit MockableCursor(const MockableData& records, bool pass) : m_cursor(records.begin()), m_cursor_end(records.end()), m_pass(pass) {} + MockableCursor(const MockableData& records, bool pass, Span prefix); ~MockableCursor() {} Status Next(DataStream& key, DataStream& value) override; @@ -68,7 +71,7 @@ class MockableCursor: public DatabaseCursor class MockableBatch : public DatabaseBatch { private: - std::map& m_records; + MockableData& m_records; bool m_pass; bool ReadKey(DataStream&& key, DataStream& value) override; @@ -78,7 +81,7 @@ class MockableBatch : public DatabaseBatch bool ErasePrefix(Span prefix) override; public: - explicit MockableBatch(std::map& records, bool pass) : m_records(records), m_pass(pass) {} + explicit MockableBatch(MockableData& records, bool pass) : m_records(records), m_pass(pass) {} ~MockableBatch() {} void Flush() override {} @@ -88,6 +91,9 @@ class MockableBatch : public DatabaseBatch { return std::make_unique(m_records, m_pass); } + std::unique_ptr GetNewPrefixCursor(Span prefix) override { + return std::make_unique(m_records, m_pass, prefix); + } bool TxnBegin() override { return m_pass; } bool TxnCommit() override { return m_pass; } bool TxnAbort() override { return m_pass; } @@ -98,10 +104,10 @@ class MockableBatch : public DatabaseBatch class MockableDatabase : public WalletDatabase { public: - std::map m_records; + MockableData m_records; bool m_pass{true}; - MockableDatabase(std::map records = {}) : WalletDatabase(), m_records(records) {} + MockableDatabase(MockableData records = {}) : WalletDatabase(), m_records(records) {} ~MockableDatabase() {}; void Open() override {} @@ -122,7 +128,7 @@ class MockableDatabase : public WalletDatabase std::unique_ptr MakeBatch(bool flush_on_close = true) override { return std::make_unique(m_records, m_pass); } }; -std::unique_ptr CreateMockableWalletDatabase(std::map records = {}); +std::unique_ptr CreateMockableWalletDatabase(MockableData records = {}); MockableDatabase& GetMockableDatabase(CWallet& wallet); } // namespace wallet diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index dfec1db4d420..fc82abcaf9da 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -1730,7 +1730,7 @@ BOOST_FIXTURE_TEST_CASE(UnconfirmableOutputsAreNotWalletFunds, ListCoinsTestingS LOCK(wallet->cs_wallet); const auto dest{wallet->GetNewDestination("")}; - BOOST_ASSERT(dest); + BOOST_REQUIRE(dest); // Use a real CoinJoin denomination so the denominated-credit paths apply. const CAmount denom{CoinJoin::GetSmallestDenomination()}; @@ -1760,7 +1760,7 @@ BOOST_FIXTURE_TEST_CASE(MempoolRemovalInvalidatesAnonymizableTally, ListCoinsTes LOCK(wallet->cs_wallet); const auto dest{wallet->GetNewDestination("")}; - BOOST_ASSERT(dest); + BOOST_REQUIRE(dest); // A wallet transaction in the mempool is trusted at depth zero, so its outputs count // towards the anonymizable tally and that tally is cacheable. @@ -1901,7 +1901,7 @@ BOOST_FIXTURE_TEST_CASE(AbandonedSpendRestoresDustLock, ListCoinsTestingSetup) wallet->m_dust_protection_threshold = 1 * COIN; const auto dest{wallet->GetNewDestination("")}; - BOOST_ASSERT(dest); + BOOST_REQUIRE(dest); // An external transaction (no input is ours) pays us a dust-protection // target; AddToWallet() locks the output on insertion. @@ -2012,11 +2012,10 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestChain100Setup) } // Add tx to wallet - const auto& op_dest = wallet.GetNewDestination(""); - BOOST_ASSERT(op_dest); + const auto op_dest{*Assert(wallet.GetNewDestination(""))}; CMutableTransaction mtx; - mtx.vout.emplace_back(COIN, GetScriptForDestination(*op_dest)); + mtx.vout.emplace_back(COIN, GetScriptForDestination(op_dest)); mtx.vin.emplace_back(g_insecure_rand_ctx.rand256(), 0); const auto& tx_id_to_spend = wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInMempool{})->GetHash(); diff --git a/src/wallet/test/walletdb_tests.cpp b/src/wallet/test/walletdb_tests.cpp index 94fb2a5ca1b2..75b49da43ed2 100644 --- a/src/wallet/test/walletdb_tests.cpp +++ b/src/wallet/test/walletdb_tests.cpp @@ -160,7 +160,7 @@ BOOST_AUTO_TEST_CASE(walletdb_read_write_deadlock) DatabaseStatus status; bilingual_str error_string; std::unique_ptr db = MakeDatabase(m_path_root / strprintf("wallet_%d_.dat", db_format).c_str(), options, status, error_string); - BOOST_ASSERT(status == DatabaseStatus::SUCCESS); + BOOST_REQUIRE(status == DatabaseStatus::SUCCESS); std::shared_ptr wallet(new CWallet(m_node.chain.get(), "", std::move(db))); wallet->m_keypool_size = 4; diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index aa991f43521c..aa8893be5f84 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -83,7 +83,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_ckey, TestingSetup) { SerializeData ckey_record_key; SerializeData ckey_record_value; - std::map records; + MockableData records; { // Context setup. @@ -169,7 +169,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_ckey, TestingSetup) // Fourth test case: // Verify that loading up a 'ckey' with an invalid pubkey throws an error CPubKey invalid_key; - BOOST_ASSERT(!invalid_key.IsValid()); + BOOST_CHECK(!invalid_key.IsValid()); SerializeData key = MakeSerializeData(DBKeys::CRYPTED_KEY, invalid_key); records[key] = ckey_record_value; diff --git a/test/config.ini.in b/test/config.ini.in index 89d3e6995d41..28ab84ea6c6e 100644 --- a/test/config.ini.in +++ b/test/config.ini.in @@ -22,7 +22,7 @@ RPCAUTH=@abs_top_srcdir@/share/rpcauth/rpcauth.py @BUILD_BITCOIN_UTIL_TRUE@ENABLE_UTIL_TOOL=true @BUILD_BITCOIN_WALLET_TRUE@ENABLE_WALLET_TOOL=true @BUILD_BITCOIND_TRUE@ENABLE_BITCOIND=true -@ENABLE_FUZZ_TRUE@ENABLE_FUZZ=true +@ENABLE_FUZZ_BINARY_TRUE@ENABLE_FUZZ_BINARY=true @ENABLE_ZMQ_TRUE@ENABLE_ZMQ=true @ENABLE_EXTERNAL_SIGNER_TRUE@ENABLE_EXTERNAL_SIGNER=true @ENABLE_USDT_TRACEPOINTS_TRUE@ENABLE_USDT_TRACEPOINTS=true diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index 191700baf99c..fad3c5833e36 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -14,7 +14,6 @@ from test_framework.messages import CBlock, from_hex, uint256_to_string from test_framework.test_framework import ( DashTestFramework, - MasternodeInfo, ) from test_framework.governance import EXPECTED_STDERR_GOV_PRUNED_TOO_FAR, have_trigger_for_height, prepare_object from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync, satoshi_round @@ -252,7 +251,7 @@ def run_test(self): _, mn_payee_protx = height_protx_list[1] payee_idx = None - for mn in self.mninfo: # type: MasternodeInfo + for mn in self.mninfo: if mn.proTxHash == mn_payee_protx: payee_idx = mn.nodeIdx break diff --git a/test/functional/feature_llmq_connections.py b/test/functional/feature_llmq_connections.py index b40d326fa285..663586460bd7 100755 --- a/test/functional/feature_llmq_connections.py +++ b/test/functional/feature_llmq_connections.py @@ -14,7 +14,6 @@ from test_framework.test_framework import ( DashTestFramework, - MasternodeInfo, ) from test_framework.util import assert_greater_than_or_equal, force_finish_mnsync @@ -86,7 +85,7 @@ def run_test(self): self.log.info("check that old masternode connections are dropped") removed = False - for mn in self.mninfo: # type: MasternodeInfo + for mn in self.mninfo: if len(mn.get_node(self).quorum("memberof", mn.proTxHash)) > 0: try: with mn.get_node(self).assert_debug_log(['removing masternodes quorum connections']): @@ -102,7 +101,7 @@ def run_test(self): self.log.info("check that inter-quorum masternode connections are added") added = False - for mn in self.mninfo: # type: MasternodeInfo + for mn in self.mninfo: if len(mn.get_node(self).quorum("memberof", mn.proTxHash)) > 0: try: with mn.get_node(self).assert_debug_log(['adding mn inter-quorum connections']): @@ -199,11 +198,11 @@ def check_mn_peers(node=mn_info.get_node(self), others=others): def check_reconnects(self, expected_connection_count): self.log.info("disable and re-enable networking on all masternodes") - for mn in self.mninfo: # type: MasternodeInfo + for mn in self.mninfo: mn.get_node(self).setnetworkactive(False) - for mn in self.mninfo: # type: MasternodeInfo + for mn in self.mninfo: self.wait_until(lambda: len(mn.get_node(self).getpeerinfo()) == 0) - for mn in self.mninfo: # type: MasternodeInfo + for mn in self.mninfo: mn.get_node(self).setnetworkactive(True) self.bump_mocktime(60) diff --git a/test/functional/feature_llmq_singlenode.py b/test/functional/feature_llmq_singlenode.py index 4e7040b85994..0238ddced9b2 100755 --- a/test/functional/feature_llmq_singlenode.py +++ b/test/functional/feature_llmq_singlenode.py @@ -14,7 +14,6 @@ from test_framework.authproxy import JSONRPCException from test_framework.test_framework import ( DashTestFramework, - MasternodeInfo, ) from test_framework.util import ( assert_raises_rpc_error, @@ -43,7 +42,7 @@ def check_sigs(self, hasrecsigs, isconflicting1, isconflicting2): conflicting_1 = False conflicting_2 = False - for mn in self.mninfo: # type: MasternodeInfo + for mn in self.mninfo: if mn.get_node(self).quorum("hasrecsig", q_type, id, msgHash): has_sig = True if mn.get_node(self).quorum("isconflicting", q_type, id, msgHash): diff --git a/test/functional/feature_mnehf.py b/test/functional/feature_mnehf.py index 7ae66bc23f97..10995fc43c2a 100755 --- a/test/functional/feature_mnehf.py +++ b/test/functional/feature_mnehf.py @@ -17,7 +17,6 @@ from test_framework.test_framework import ( DashTestFramework, - MasternodeInfo ) from test_framework.util import ( assert_equal, @@ -46,7 +45,7 @@ def restart_all_nodes(self, params=None): self.log.info(f"Actual restart options: {self.extra_args[inode]}") self.restart_node(0) - for mn in self.mninfo: # type: MasternodeInfo + for mn in self.mninfo: index = mn.nodeIdx self.stop_node(index) self.start_masternode(mn) diff --git a/test/functional/interface_zmq_dash.py b/test/functional/interface_zmq_dash.py index a97b49e1cb30..fa9782ea49e8 100755 --- a/test/functional/interface_zmq_dash.py +++ b/test/functional/interface_zmq_dash.py @@ -14,7 +14,6 @@ from test_framework.test_framework import ( DashTestFramework, - MasternodeInfo, ) from test_framework.p2p import P2PInterface from test_framework.util import ( @@ -219,7 +218,7 @@ def validate_recovered_sig(request_id, msg_hash): sign_id = uint256_to_string(random.getrandbits(256)) sign_msg_hash = uint256_to_string(random.getrandbits(256)) quorumHash = self.nodes[0].quorum("selectquorum", 100, sign_id)["quorumHash"] - for mn in self.get_quorum_masternodes(quorumHash): # type: MasternodeInfo + for mn in self.get_quorum_masternodes(quorumHash): mn.get_node(self).quorum("sign", 100, sign_id, sign_msg_hash) validate_recovered_sig(sign_id, sign_msg_hash) # Unsubscribe from recovered signature messages diff --git a/test/functional/p2p_invalid_locator.py b/test/functional/p2p_invalid_locator.py index a586b48d4c00..2a521ec0d7a2 100755 --- a/test/functional/p2p_invalid_locator.py +++ b/test/functional/p2p_invalid_locator.py @@ -32,7 +32,7 @@ def run_test(self): within_max_peer = node.add_p2p_connection(P2PInterface()) msg.locator.vHave = [int(node.getblockhash(i - 1), 16) for i in range(block_count, block_count - (MAX_LOCATOR_SZ), -1)] within_max_peer.send_message(msg) - if type(msg) == msg_getheaders: + if type(msg) is msg_getheaders: within_max_peer.wait_for_header(node.getbestblockhash()) else: within_max_peer.wait_for_block(int(node.getbestblockhash(), 16)) diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py index 8f76302717eb..714115c640e0 100755 --- a/test/functional/rpc_help.py +++ b/test/functional/rpc_help.py @@ -87,8 +87,8 @@ def test_client_conversion_table(self): for argname, convert in converts_by_argname.items(): if all(convert) != any(convert): - # Only allow dummy to fail consistency check - assert argname == 'dummy', ('WARNING: conversion mismatch for argument named %s (%s)' % (argname, list(zip(all_methods_by_argname[argname], converts_by_argname[argname])))) + # Only allow dummy and psbt to fail consistency check + assert argname in ['dummy', "psbt"], ('WARNING: conversion mismatch for argument named %s (%s)' % (argname, list(zip(all_methods_by_argname[argname], converts_by_argname[argname])))) def test_categories(self): node = self.nodes[0] diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 1ad6ede8b65e..58490bf8724b 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -329,7 +329,7 @@ def run_test(self): self.log.info("Test various PSBT operations") # partially sign multisig things with node 1 - psbtx = wmulti.walletcreatefundedpsbt(inputs=[{"txid":txid,"vout":p2sh_pos}], outputs={self.nodes[1].getnewaddress():9.99}, options={'changeAddress': self.nodes[1].getrawchangeaddress()})['psbt'] + psbtx = wmulti.walletcreatefundedpsbt(inputs=[{"txid":txid,"vout":p2sh_pos}], outputs={self.nodes[1].getnewaddress():9.99}, changeAddress=self.nodes[1].getrawchangeaddress())['psbt'] walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(psbtx) psbtx = walletprocesspsbt_out['psbt'] assert_equal(walletprocesspsbt_out['complete'], False) @@ -665,7 +665,7 @@ def test_psbt_input_keys(psbt_input, keys): psbt = wallet.walletcreatefundedpsbt( inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": input_weight}], outputs={self.nodes[0].getnewaddress(): 15}, - options={"add_inputs": True} + add_inputs=True, ) signed = wallet.walletprocesspsbt(psbt["psbt"]) signed = self.nodes[0].walletprocesspsbt(signed["psbt"]) @@ -675,21 +675,21 @@ def test_psbt_input_keys(psbt_input, keys): psbt2 = wallet.walletcreatefundedpsbt( inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": low_input_weight}], outputs={self.nodes[0].getnewaddress(): 15}, - options={"add_inputs": True} + add_inputs=True, ) assert_greater_than(psbt["fee"], psbt2["fee"]) # Increasing the weight should have a higher fee psbt2 = wallet.walletcreatefundedpsbt( inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}], outputs={self.nodes[0].getnewaddress(): 15}, - options={"add_inputs": True} + add_inputs=True, ) assert_greater_than(psbt2["fee"], psbt["fee"]) # The provided weight should override the calculated weight when solving data is provided psbt3 = wallet.walletcreatefundedpsbt( inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}], outputs={self.nodes[0].getnewaddress(): 15}, - options={'add_inputs': True, "solving_data":{"descriptors": [desc]}} + add_inputs=True, solving_data={"descriptors": [desc]}, ) assert_equal(psbt2["fee"], psbt3["fee"]) @@ -703,7 +703,7 @@ def test_psbt_input_keys(psbt_input, keys): psbt3 = wallet.walletcreatefundedpsbt( inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}], outputs={self.nodes[0].getnewaddress(): 15}, - options={"add_inputs": True} + add_inputs=True, ) assert_equal(psbt2["fee"], psbt3["fee"]) diff --git a/test/functional/test_framework/crypto/siphash.py b/test/functional/test_framework/crypto/siphash.py index 884dbcab4694..bd13b2c948d8 100644 --- a/test/functional/test_framework/crypto/siphash.py +++ b/test/functional/test_framework/crypto/siphash.py @@ -31,7 +31,7 @@ def siphash_round(v0, v1, v2, v3): def siphash(k0, k1, data): - assert type(data) == bytes + assert type(data) is bytes v0 = 0x736f6d6570736575 ^ k0 v1 = 0x646f72616e646f6d ^ k1 v2 = 0x6c7967656e657261 ^ k0 @@ -61,5 +61,5 @@ def siphash(k0, k1, data): def siphash256(k0, k1, num): - assert type(num) == int + assert type(num) is int return siphash(k0, k1, num.to_bytes(32, 'little')) diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index f6b5eaf948e3..4530ac92d051 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -146,7 +146,7 @@ def test_change_position(self): """Ensure setting changePosition in fundraw with an exact match is handled properly.""" self.log.info("Test fundrawtxn changePosition option") rawmatch = self.nodes[2].createrawtransaction([], {self.nodes[2].getnewaddress():500}) - rawmatch = self.nodes[2].fundrawtransaction(rawmatch, {"changePosition":1, "subtractFeeFromOutputs":[0]}) + rawmatch = self.nodes[2].fundrawtransaction(rawmatch, changePosition=1, subtractFeeFromOutputs=[0]) assert_equal(rawmatch["changepos"], -1) self.nodes[3].createwallet(wallet_name="wwatch", disable_private_keys=True) @@ -257,10 +257,10 @@ def test_invalid_option(self): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - assert_raises_rpc_error(-3, "Unexpected key foo", self.nodes[2].fundrawtransaction, rawtx, {'foo':'bar'}) + assert_raises_rpc_error(-8, "Unknown named parameter foo", self.nodes[2].fundrawtransaction, rawtx, foo='bar') # reserveChangeKey was deprecated and is now removed - assert_raises_rpc_error(-3, "Unexpected key reserveChangeKey", lambda: self.nodes[2].fundrawtransaction(hexstring=rawtx, options={'reserveChangeKey': True})) + assert_raises_rpc_error(-8, "Unknown named parameter reserveChangeKey", lambda: self.nodes[2].fundrawtransaction(hexstring=rawtx, reserveChangeKey=True)) def test_invalid_change_address(self): self.log.info("Test fundrawtxn with an invalid change address") @@ -272,7 +272,7 @@ def test_invalid_change_address(self): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - assert_raises_rpc_error(-5, "Change address must be a valid Dash address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'}) + assert_raises_rpc_error(-5, "Change address must be a valid Dash address", self.nodes[2].fundrawtransaction, rawtx, changeAddress='foobar') def test_valid_change_address(self): self.log.info("Test fundrawtxn with a provided change address") @@ -285,8 +285,8 @@ def test_valid_change_address(self): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) change = self.nodes[2].getnewaddress() - assert_raises_rpc_error(-8, "changePosition out of bounds", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':change, 'changePosition':2}) - rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {'changeAddress': change, 'changePosition': 0}) + assert_raises_rpc_error(-8, "changePosition out of bounds", self.nodes[2].fundrawtransaction, rawtx, changeAddress=change, changePosition=2) + rawtxfund = self.nodes[2].fundrawtransaction(rawtx, changeAddress=change, changePosition=0) dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) out = dec_tx['vout'][0] assert_equal(change, out['scriptPubKey']['address']) @@ -307,7 +307,7 @@ def test_coin_selection(self): assert_equal("00", dec_tx['vin'][0]['scriptSig']['hex']) # Should fail without add_inputs: - assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, add_inputs=False) # add_inputs is enabled by default rawtxfund = self.nodes[2].fundrawtransaction(rawtx) @@ -337,8 +337,8 @@ def test_two_vin(self): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) # Should fail without add_inputs: - assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) - rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True}) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, add_inputs=False) + rawtxfund = self.nodes[2].fundrawtransaction(rawtx, add_inputs=True) dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) matchingOuts = 0 @@ -369,8 +369,8 @@ def test_two_vin_two_vout(self): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) # Should fail without add_inputs: - assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) - rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True}) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, self.nodes[2].fundrawtransaction, rawtx, add_inputs=False) + rawtxfund = self.nodes[2].fundrawtransaction(rawtx, add_inputs=True) dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) matchingOuts = 0 @@ -534,7 +534,7 @@ def test_spend_2of2(self): oldBalance = self.nodes[1].getbalance() inputs = [] outputs = {self.nodes[1].getnewaddress():11} - funded_psbt = wmulti.walletcreatefundedpsbt(inputs=inputs, outputs=outputs, options={'changeAddress': w2.getrawchangeaddress()})['psbt'] + funded_psbt = wmulti.walletcreatefundedpsbt(inputs=inputs, outputs=outputs, changeAddress=w2.getrawchangeaddress())['psbt'] signed_psbt = w2.walletprocesspsbt(funded_psbt) self.nodes[2].sendrawtransaction(signed_psbt['hex']) @@ -736,7 +736,7 @@ def test_all_watched_funds(self): self.nodes[3].loadwallet('wwatch') wwatch = self.nodes[3].get_wallet_rpc('wwatch') w3 = self.nodes[3].get_wallet_rpc(self.default_wallet_name) - result = wwatch.fundrawtransaction(rawtx, {'includeWatching': True, 'changeAddress': w3.getrawchangeaddress(), 'subtractFeeFromOutputs': [0]}) + result = wwatch.fundrawtransaction(rawtx, includeWatching=True, changeAddress=w3.getrawchangeaddress(), subtractFeeFromOutputs=[0]) res_dec = self.nodes[0].decoderawtransaction(result["hex"]) assert_equal(len(res_dec["vin"]), 1) assert res_dec["vin"][0]["txid"] == self.watchonly_txid @@ -765,10 +765,10 @@ def test_option_feerate(self): result = node.fundrawtransaction(rawtx) # uses self.min_relay_tx_fee (set by settxfee) btc_kvb_to_sat_vb = 100000 # (1e5) - result1 = node.fundrawtransaction(rawtx, {"fee_rate": str(2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee)}) - result2 = node.fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}) - result3 = node.fundrawtransaction(rawtx, {"fee_rate": 10 * btc_kvb_to_sat_vb * self.min_relay_tx_fee}) - result4 = node.fundrawtransaction(rawtx, {"feeRate": str(10 * self.min_relay_tx_fee)}) + result1 = node.fundrawtransaction(rawtx, fee_rate=str(2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee)) + result2 = node.fundrawtransaction(rawtx, feeRate=2 * self.min_relay_tx_fee) + result3 = node.fundrawtransaction(rawtx, fee_rate=10 * btc_kvb_to_sat_vb * self.min_relay_tx_fee) + result4 = node.fundrawtransaction(rawtx, feeRate=str(10 * self.min_relay_tx_fee)) result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex']) assert_fee_amount(result1['fee'], count_bytes(result1['hex']), 2 * result_fee_rate) @@ -783,54 +783,54 @@ def test_option_feerate(self): # With no arguments passed, expect fee of 225 satoshis. assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000225, vspan=0.00000001) # Expect fee to be 10,000x higher when an explicit fee rate 10,000x greater is specified. - result = node.fundrawtransaction(rawtx, {"fee_rate": 10000}) + result = node.fundrawtransaction(rawtx, fee_rate=10000) assert_approx(result["fee"], vexp=0.0225, vspan=0.0001) self.log.info("Test fundrawtxn with invalid estimate_mode settings") for k, v in {"number": 42, "object": {"foo": "bar"}}.items(): assert_raises_rpc_error(-3, f"JSON value of type {k} for field estimate_mode is not of expected type string", - node.fundrawtransaction, rawtx, {"estimate_mode": v, "conf_target": 0.1, "add_inputs": True}) + node.fundrawtransaction, rawtx, estimate_mode=v, conf_target=0.1, add_inputs=True) for mode in ["", "foo", Decimal("3.141592")]: assert_raises_rpc_error(-8, 'Invalid estimate_mode parameter, must be one of: "unset", "economical", "conservative"', - node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": 0.1, "add_inputs": True}) + node.fundrawtransaction, rawtx, estimate_mode=mode, conf_target=0.1, add_inputs=True) self.log.info("Test fundrawtxn with invalid conf_target settings") for mode in ["unset", "economical", "conservative"]: self.log.debug("{}".format(mode)) for k, v in {"string": "", "object": {"foo": "bar"}}.items(): assert_raises_rpc_error(-3, f"JSON value of type {k} for field conf_target is not of expected type number", - node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": v, "add_inputs": True}) + node.fundrawtransaction, rawtx, estimate_mode=mode, conf_target=v, add_inputs=True) for n in [-1, 0, 1009]: assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", # max value of 1008 per src/policy/fees.h - node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": n, "add_inputs": True}) + node.fundrawtransaction, rawtx, estimate_mode=mode, conf_target=n, add_inputs=True) self.log.info("Test invalid fee rate settings") for param, value in {("fee_rate", 100000), ("feeRate", 1.000)}: assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", - node.fundrawtransaction, rawtx, {param: value, "add_inputs": True}) + node.fundrawtransaction, rawtx, add_inputs=True, **{param: value}) assert_raises_rpc_error(-3, "Amount out of range", - node.fundrawtransaction, rawtx, {param: -1, "add_inputs": True}) + node.fundrawtransaction, rawtx, add_inputs=True, **{param: -1}) assert_raises_rpc_error(-3, "Amount is not a number or string", - node.fundrawtransaction, rawtx, {param: {"foo": "bar"}, "add_inputs": True}) + node.fundrawtransaction, rawtx, add_inputs=True, **{param: {"foo": "bar"}}) # Test fee rate values that don't pass fixed-point parsing checks. for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]: - assert_raises_rpc_error(-3, "Invalid amount", node.fundrawtransaction, rawtx, {param: invalid_value, "add_inputs": True}) + assert_raises_rpc_error(-3, "Invalid amount", node.fundrawtransaction, rawtx, add_inputs=True, **{param: invalid_value}) # Test fee_rate values that cannot be represented in sat/vB. for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]: assert_raises_rpc_error(-3, "Invalid amount", - node.fundrawtransaction, rawtx, {"fee_rate": invalid_value, "add_inputs": True}) + node.fundrawtransaction, rawtx, fee_rate=invalid_value, add_inputs=True) self.log.info("Test min fee rate checks are bypassed with fundrawtxn, e.g. a fee_rate under 1 sat/vB is allowed") - node.fundrawtransaction(rawtx, {"fee_rate": 0.999, "add_inputs": True}) - node.fundrawtransaction(rawtx, {"feeRate": 0.00000999, "add_inputs": True}) + node.fundrawtransaction(rawtx, fee_rate=0.999, add_inputs=True) + node.fundrawtransaction(rawtx, feeRate=0.00000999, add_inputs=True) self.log.info("- raises RPC error if both feeRate and fee_rate are passed") assert_raises_rpc_error(-8, "Cannot specify both fee_rate (duff/B) and feeRate (DASH/kB)", - node.fundrawtransaction, rawtx, {"fee_rate": 0.1, "feeRate": 0.1, "add_inputs": True}) + node.fundrawtransaction, rawtx, fee_rate=0.1, feeRate=0.1, add_inputs=True) self.log.info("- raises RPC error if both feeRate and estimate_mode passed") assert_raises_rpc_error(-8, "Cannot specify both estimate_mode and feeRate", - node.fundrawtransaction, rawtx, {"estimate_mode": "economical", "feeRate": 0.1, "add_inputs": True}) + node.fundrawtransaction, rawtx, estimate_mode="economical", feeRate=0.1, add_inputs=True) for param in ["feeRate", "fee_rate"]: self.log.info("- raises RPC error if both {} and conf_target are passed".format(param)) @@ -840,7 +840,7 @@ def test_option_feerate(self): self.log.info("- raises RPC error if both fee_rate and estimate_mode are passed") assert_raises_rpc_error(-8, "Cannot specify both estimate_mode and fee_rate", - node.fundrawtransaction, rawtx, {"fee_rate": 1, "estimate_mode": "economical", "add_inputs": True}) + node.fundrawtransaction, rawtx, fee_rate=1, estimate_mode="economical", add_inputs=True) def test_address_reuse(self): """Test no address reuse occurs.""" @@ -870,10 +870,10 @@ def test_option_subtract_fee_from_outputs(self): # Test subtract fee from outputs with feeRate (BTC/kvB) result = [self.nodes[3].fundrawtransaction(rawtx), # uses self.min_relay_tx_fee (set by settxfee) - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses self.min_relay_tx_fee (set by settxfee) - self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}), - self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee, "subtractFeeFromOutputs": [0]}),] + self.nodes[3].fundrawtransaction(rawtx, subtractFeeFromOutputs=[]), # empty subtraction list + self.nodes[3].fundrawtransaction(rawtx, subtractFeeFromOutputs=[0]), # uses self.min_relay_tx_fee (set by settxfee) + self.nodes[3].fundrawtransaction(rawtx, feeRate=2 * self.min_relay_tx_fee), + self.nodes[3].fundrawtransaction(rawtx, feeRate=2 * self.min_relay_tx_fee, subtractFeeFromOutputs=[0]),] dec_tx = [self.nodes[3].decoderawtransaction(tx_['hex']) for tx_ in result] output = [d['vout'][1 - r['changepos']]['value'] for d, r in zip(dec_tx, result)] change = [d['vout'][r['changepos']]['value'] for d, r in zip(dec_tx, result)] @@ -890,10 +890,10 @@ def test_option_subtract_fee_from_outputs(self): # Test subtract fee from outputs with fee_rate (sat/vB) btc_kvb_to_sat_vb = 100000 # (1e5) result = [self.nodes[3].fundrawtransaction(rawtx), # uses self.min_relay_tx_fee (set by settxfee) - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses self.min_relay_tx_fee (set by settxfee) - self.nodes[3].fundrawtransaction(rawtx, {"fee_rate": 2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee}), - self.nodes[3].fundrawtransaction(rawtx, {"fee_rate": 2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee, "subtractFeeFromOutputs": [0]}),] + self.nodes[3].fundrawtransaction(rawtx, subtractFeeFromOutputs=[]), # empty subtraction list + self.nodes[3].fundrawtransaction(rawtx, subtractFeeFromOutputs=[0]), # uses self.min_relay_tx_fee (set by settxfee) + self.nodes[3].fundrawtransaction(rawtx, fee_rate=2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee), + self.nodes[3].fundrawtransaction(rawtx, fee_rate=2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee, subtractFeeFromOutputs=[0]),] dec_tx = [self.nodes[3].decoderawtransaction(tx_['hex']) for tx_ in result] output = [d['vout'][1 - r['changepos']]['value'] for d, r in zip(dec_tx, result)] change = [d['vout'][r['changepos']]['value'] for d, r in zip(dec_tx, result)] @@ -914,7 +914,7 @@ def test_option_subtract_fee_from_outputs(self): # Add changePosition=4 to circumvent BIP69 input/output sorting result = [self.nodes[3].fundrawtransaction(rawtx, {"changePosition": 4}), # Split the fee between outputs 0, 2, and 3, but not output 1. - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0, 2, 3], "changePosition": 4})] + self.nodes[3].fundrawtransaction(rawtx, subtractFeeFromOutputs=[0, 2, 3], changePosition=4)] dec_tx = [self.nodes[3].decoderawtransaction(result[0]['hex']), self.nodes[3].decoderawtransaction(result[1]['hex'])] @@ -956,7 +956,7 @@ def test_subtract_fee_with_presets(self): vout = find_vout_for_address(self.nodes[0], txid, addr) rawtx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], [{self.nodes[0].getnewaddress(): 5}]) - fundedtx = self.nodes[0].fundrawtransaction(rawtx, {'subtractFeeFromOutputs': [0]}) + fundedtx = self.nodes[0].fundrawtransaction(rawtx, subtractFeeFromOutputs=[0]) signedtx = self.nodes[0].signrawtransactionwithwallet(fundedtx['hex']) self.nodes[0].sendrawtransaction(signedtx['hex']) @@ -990,25 +990,25 @@ def test_external_inputs(self): assert_raises_rpc_error(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"], ext_utxo["vout"]), wallet.fundrawtransaction, raw_tx) # Error conditions - assert_raises_rpc_error(-5, 'Pubkey "not a pubkey" must be a hex string', wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["not a pubkey"]}}) - assert_raises_rpc_error(-5, 'Pubkey "01234567890a0b0c0d0e0f" must have a length of either 33 or 65 bytes', wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["01234567890a0b0c0d0e0f"]}}) - assert_raises_rpc_error(-5, "'not a script' is not hex", wallet.fundrawtransaction, raw_tx, {"solving_data": {"scripts":["not a script"]}}) - assert_raises_rpc_error(-8, "Unable to parse descriptor 'not a descriptor'", wallet.fundrawtransaction, raw_tx, {"solving_data": {"descriptors":["not a descriptor"]}}) - assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", wallet.fundrawtransaction, raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"]}]}) - assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", wallet.fundrawtransaction, raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": -1}]}) - assert_raises_rpc_error(-8, "Invalid parameter, missing size key", wallet.fundrawtransaction, raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"]}]}) - assert_raises_rpc_error(-8, "Invalid parameter, size cannot be less than 41", wallet.fundrawtransaction, raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": 40}]}) - assert_raises_rpc_error(-8, "Invalid parameter, size cannot be less than 41", wallet.fundrawtransaction, raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": -1}]}) - assert_raises_rpc_error(-8, "Invalid parameter, size cannot be greater than", wallet.fundrawtransaction, raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": 400001}]}) + assert_raises_rpc_error(-5, 'Pubkey "not a pubkey" must be a hex string', wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["not a pubkey"]}) + assert_raises_rpc_error(-5, 'Pubkey "01234567890a0b0c0d0e0f" must have a length of either 33 or 65 bytes', wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["01234567890a0b0c0d0e0f"]}) + assert_raises_rpc_error(-5, "'not a script' is not hex", wallet.fundrawtransaction, raw_tx, solving_data={"scripts":["not a script"]}) + assert_raises_rpc_error(-8, "Unable to parse descriptor 'not a descriptor'", wallet.fundrawtransaction, raw_tx, solving_data={"descriptors":["not a descriptor"]}) + assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", wallet.fundrawtransaction, raw_tx, input_sizes=[{"txid": ext_utxo["txid"]}]) + assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", wallet.fundrawtransaction, raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": -1}]) + assert_raises_rpc_error(-8, "Invalid parameter, missing size key", wallet.fundrawtransaction, raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"]}]) + assert_raises_rpc_error(-8, "Invalid parameter, size cannot be less than 41", wallet.fundrawtransaction, raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": 40}]) + assert_raises_rpc_error(-8, "Invalid parameter, size cannot be less than 41", wallet.fundrawtransaction, raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": -1}]) + assert_raises_rpc_error(-8, "Invalid parameter, size cannot be greater than", wallet.fundrawtransaction, raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": 400001}]) # But funding should work when the solving data is provided - funded_tx = wallet.fundrawtransaction(raw_tx, {"solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}}) + funded_tx = wallet.fundrawtransaction(raw_tx, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}) signed_tx = wallet.signrawtransactionwithwallet(funded_tx['hex']) assert not signed_tx['complete'] signed_tx = self.nodes[0].signrawtransactionwithwallet(signed_tx['hex']) assert signed_tx['complete'] - funded_tx = wallet.fundrawtransaction(raw_tx, {"solving_data": {"descriptors": [desc]}}) + funded_tx = wallet.fundrawtransaction(raw_tx, solving_data={"descriptors": [desc]}) signed_tx1 = wallet.signrawtransactionwithwallet(funded_tx['hex']) assert not signed_tx1['complete'] signed_tx2 = self.nodes[0].signrawtransactionwithwallet(signed_tx1['hex']) @@ -1023,29 +1023,29 @@ def test_external_inputs(self): high_input_weight = input_weight * 2 # Funding should also work if the input weight is provided - funded_tx = wallet.fundrawtransaction(raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": input_weight}]}) + funded_tx = wallet.fundrawtransaction(raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": input_weight}]) signed_tx = wallet.signrawtransactionwithwallet(funded_tx["hex"]) signed_tx = self.nodes[0].signrawtransactionwithwallet(signed_tx["hex"]) assert_equal(self.nodes[0].testmempoolaccept([signed_tx["hex"]])[0]["allowed"], True) assert_equal(signed_tx["complete"], True) # Reducing the weight should have a lower fee - funded_tx2 = wallet.fundrawtransaction(raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": low_input_weight}]}) + funded_tx2 = wallet.fundrawtransaction(raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": low_input_weight}]) assert_greater_than(funded_tx["fee"], funded_tx2["fee"]) # Increasing the weight should have a higher fee - funded_tx2 = wallet.fundrawtransaction(raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}]}) + funded_tx2 = wallet.fundrawtransaction(raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}]) assert_greater_than(funded_tx2["fee"], funded_tx["fee"]) # The provided weight should override the calculated weight when solving data is provided - funded_tx3 = wallet.fundrawtransaction(raw_tx, {"solving_data": {"descriptors": [desc]}, "input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}]}) + funded_tx3 = wallet.fundrawtransaction(raw_tx, solving_data={"descriptors": [desc]}, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}]) assert_equal(funded_tx2["fee"], funded_tx3["fee"]) # The feerate should be met - funded_tx4 = wallet.fundrawtransaction(raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}], "fee_rate": 10}) + funded_tx4 = wallet.fundrawtransaction(raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": high_input_weight}], fee_rate=10) input_add_weight = high_input_weight - 41 tx4_size = wallet.decoderawtransaction(funded_tx4["hex"])["size"] + input_add_weight assert_fee_amount(funded_tx4["fee"], tx4_size, Decimal(0.0001)) # Funding with weight at csuint boundaries should not cause problems - funded_tx = wallet.fundrawtransaction(raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": 255}]}) - funded_tx = wallet.fundrawtransaction(raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": 65539}]}) + funded_tx = wallet.fundrawtransaction(raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": 255}]) + funded_tx = wallet.fundrawtransaction(raw_tx, input_sizes=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": 65539}]) self.nodes[2].unloadwallet("extfund") @@ -1085,7 +1085,7 @@ def test_add_inputs_default_value(self): # Fund wallet with 2 outputs, 5 BTC each. addr2 = wallet.getnewaddress() - source_tx = self.nodes[0].send(outputs=[{addr1: 5}, {addr2: 5}], options={"change_position": 0}) + source_tx = self.nodes[0].send(outputs=[{addr1: 5}, {addr2: 5}], change_position=0) self.generate(self.nodes[0], 1) # Select only one input. @@ -1097,12 +1097,12 @@ def test_add_inputs_default_value(self): } ] } - assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.send, outputs=[{addr1: 8}], options=options) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.send, outputs=[{addr1: 8}], **options) # Case (3), Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount) options["add_inputs"] = True options["add_to_wallet"] = False - tx = wallet.send(outputs=[{addr1: 8}], options=options) + tx = wallet.send(outputs=[{addr1: 8}], **options) assert tx["complete"] # Case (4), Explicit add_inputs=true and preset inputs (with preset inputs covering the target amount) @@ -1110,7 +1110,7 @@ def test_add_inputs_default_value(self): "txid": source_tx["txid"], "vout": 2 # change position was hardcoded to index 0 }) - tx = wallet.send(outputs=[{addr1: 8}], options=options) + tx = wallet.send(outputs=[{addr1: 8}], **options) assert tx["complete"] # Check that only the preset inputs were added to the tx decoded_psbt_inputs = self.nodes[0].decodepsbt(tx["psbt"])['tx']['vin'] @@ -1120,12 +1120,12 @@ def test_add_inputs_default_value(self): # Case (5), assert that inputs are added to the tx by explicitly setting add_inputs=true options = {"add_inputs": True, "add_to_wallet": True} - tx = wallet.send(outputs=[{addr1: 8}], options=options) + tx = wallet.send(outputs=[{addr1: 8}], **options) assert tx["complete"] # 6. Explicit add_inputs=false, no preset inputs: options = {"add_inputs": False} - assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.send, outputs=[{addr1: 3}], options=options) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.send, outputs=[{addr1: 3}], **options) ################################################ @@ -1146,14 +1146,14 @@ def test_add_inputs_default_value(self): # Case (3), Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount) options["add_inputs"] = True - assert "psbt" in wallet.walletcreatefundedpsbt(outputs=[{addr1: 8}], inputs=inputs, options=options) + assert "psbt" in wallet.walletcreatefundedpsbt(outputs=[{addr1: 8}], inputs=inputs, **options) # Case (4), Explicit add_inputs=true and preset inputs (with preset inputs covering the target amount) inputs.append({ "txid": source_tx["txid"], "vout": 2 # change position was hardcoded to index 0 }) - psbt_tx = wallet.walletcreatefundedpsbt(outputs=[{addr1: 8}], inputs=inputs, options=options) + psbt_tx = wallet.walletcreatefundedpsbt(outputs=[{addr1: 8}], inputs=inputs, **options) # Check that only the preset inputs were added to the tx decoded_psbt_inputs = self.nodes[0].decodepsbt(psbt_tx["psbt"])['tx']['vin'] assert_equal(len(decoded_psbt_inputs), 2) @@ -1165,11 +1165,11 @@ def test_add_inputs_default_value(self): options = { "add_inputs": True } - assert "psbt" in wallet.walletcreatefundedpsbt(inputs=[], outputs=outputs, options=options) + assert "psbt" in wallet.walletcreatefundedpsbt(inputs=[], outputs=outputs, **options) # Case (6). Explicit add_inputs=false, no preset inputs: options = {"add_inputs": False} - assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.walletcreatefundedpsbt, inputs=[], outputs=outputs, options=options) + assert_raises_rpc_error(-4, ERR_NOT_ENOUGH_PRESET_INPUTS, wallet.walletcreatefundedpsbt, inputs=[], outputs=outputs, **options) self.nodes[2].unloadwallet("test_preset_inputs") @@ -1237,7 +1237,7 @@ def test_include_unsafe(self): assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, rawtx) # But we can opt-in to use them for funding. - fundedtx = wallet.fundrawtransaction(rawtx, {"include_unsafe": True}) + fundedtx = wallet.fundrawtransaction(rawtx, include_unsafe=True) tx_dec = wallet.decoderawtransaction(fundedtx['hex']) assert all((txin["txid"], txin["vout"]) in inputs for txin in tx_dec["vin"]) signedtx = wallet.signrawtransactionwithwallet(fundedtx['hex']) @@ -1245,7 +1245,7 @@ def test_include_unsafe(self): # And we can also use them once they're confirmed. self.generate(self.nodes[0], 1) - fundedtx = wallet.fundrawtransaction(rawtx, {"include_unsafe": False}) + fundedtx = wallet.fundrawtransaction(rawtx, include_unsafe=False) tx_dec = wallet.decoderawtransaction(fundedtx['hex']) assert all((txin["txid"], txin["vout"]) in inputs for txin in tx_dec["vin"]) signedtx = wallet.signrawtransactionwithwallet(fundedtx['hex']) @@ -1280,7 +1280,7 @@ def test_22670(self): # Create transactions in order to calculate fees for the target bounds that can trigger this bug change_tx = tester.fundrawtransaction(tester.createrawtransaction([], [{funds.getnewaddress(): 1.5}])) tx = tester.createrawtransaction([], [{funds.getnewaddress(): 2}]) - no_change_tx = tester.fundrawtransaction(tx, {"subtractFeeFromOutputs": [0]}) + no_change_tx = tester.fundrawtransaction(tx, subtractFeeFromOutputs=[0]) overhead_fees = feerate * len(tx) / 2 / 1000 cost_of_change = change_tx["fee"] - no_change_tx["fee"] @@ -1333,7 +1333,7 @@ def test_feerate_rounding(self): # To test this does not happen, we subtract 202 sats from the input value. If working correctly, this should # fail with insufficient funds rather than bitcoind asserting. rawtx = w.createrawtransaction(inputs=[], outputs=[{self.nodes[0].getnewaddress(): 1 - 0.00000202}]) - assert_raises_rpc_error(-4, "Insufficient funds", w.fundrawtransaction, rawtx, {"fee_rate": 1.85}) + assert_raises_rpc_error(-4, "Insufficient funds", w.fundrawtransaction, rawtx, fee_rate=1.85) def test_input_confs_control(self): self.nodes[0].createwallet("minconf") diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index e32b817efe8a..fcd23630fc12 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -64,7 +64,7 @@ def do_import(self, timestamp): } response = self.node.importmulti( requests=[request], - options={"rescan": self.rescan in (Rescan.yes, Rescan.late_timestamp)}, + rescan=self.rescan in (Rescan.yes, Rescan.late_timestamp), ) assert_equal(response, [{"success": True}]) diff --git a/test/functional/wallet_keypool_hd.py b/test/functional/wallet_keypool_hd.py index cb1b5751ab80..926d3fcc25cf 100755 --- a/test/functional/wallet_keypool_hd.py +++ b/test/functional/wallet_keypool_hd.py @@ -199,30 +199,30 @@ def run_test(self): # Using a fee rate (10 sat / byte) well above the minimum relay rate # creating a 5,000 sat transaction with change should not be possible - assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it.", w2.walletcreatefundedpsbt, inputs=[], outputs=[{addr.pop(): 0.00005000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010}) + assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it.", w2.walletcreatefundedpsbt, inputs=[], outputs=[{addr.pop(): 0.00005000}], subtractFeeFromOutputs=[0], feeRate=0.000010) # creating a 10,000 sat transaction without change, with a manual input, should still be possible - res = w2.walletcreatefundedpsbt(inputs=w2.listunspent(), outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010}) + res = w2.walletcreatefundedpsbt(inputs=w2.listunspent(), outputs=[{destination: 0.00010000}], subtractFeeFromOutputs=[0], feeRate=0.000010) assert_equal("psbt" in res, True) # creating a 10,000 sat transaction without change should still be possible - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], subtractFeeFromOutputs=[0], feeRate=0.000010) assert_equal("psbt" in res, True) # should work without subtractFeeFromOutputs if the exact fee is subtracted from the amount - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008900}], options={"feeRate": 0.000010}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008900}], feeRate=0.000010) assert_equal("psbt" in res, True) # dust change should be removed - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008800}], options={"feeRate": 0.000010}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008800}], feeRate=0.000010) assert_equal("psbt" in res, True) # create a transaction without change at the maximum fee rate, such that the output is still spendable: - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.00008822}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], subtractFeeFromOutputs=[0], feeRate=0.00008822) assert_equal("psbt" in res, True) assert_equal(res["fee"], Decimal("0.00001694")) # creating a 10,000 sat transaction with a manual change address should be possible - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010, "changeAddress": addr.pop()}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], subtractFeeFromOutputs=[0], feeRate=0.000010, changeAddress=addr.pop()) assert_equal("psbt" in res, True) if not self.options.descriptors: diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 3e11e8d27836..bdfcec0b0f07 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -352,7 +352,7 @@ def test_other_watchonly(self): imports0.importaddress(import_sent_addr) received_sent_watchonly_txid = default.sendtoaddress(import_sent_addr, 10) received_sent_watchonly_vout = find_vout_for_address(self.nodes[0], received_sent_watchonly_txid, import_sent_addr) - send = default.sendall(recipients=[default.getnewaddress()], options={"inputs": [{"txid": received_sent_watchonly_txid, "vout": received_sent_watchonly_vout}]}) + send = default.sendall(recipients=[default.getnewaddress()], inputs=[{"txid": received_sent_watchonly_txid, "vout": received_sent_watchonly_vout}]) sent_watchonly_txid = send["txid"] # Tx that has both a watchonly and spendable output diff --git a/test/functional/wallet_multisig_descriptor_psbt.py b/test/functional/wallet_multisig_descriptor_psbt.py index faff0cea1fba..2959574d5b0b 100755 --- a/test/functional/wallet_multisig_descriptor_psbt.py +++ b/test/functional/wallet_multisig_descriptor_psbt.py @@ -121,7 +121,7 @@ def run_test(self): to = participants["signers"][self.N - 1].getnewaddress() value = 1 self.log.info("First, make a sending transaction, created using `walletcreatefundedpsbt` (anyone can initiate this)...") - psbt = participants["multisigs"][0].walletcreatefundedpsbt(inputs=[], outputs={to: value}, options={"feeRate": 0.00010}) + psbt = participants["multisigs"][0].walletcreatefundedpsbt(inputs=[], outputs={to: value}, feeRate=0.00010) psbts = [] self.log.info("Now at least M users check the psbt with decodepsbt and (if OK) signs it with walletprocesspsbt...") @@ -143,7 +143,7 @@ def run_test(self): assert_equal(participants["signers"][self.N - 1].getbalance(), value) self.log.info("Send another transaction from the multisig, this time with a daisy chained signing flow (one after another in series)!") - psbt = participants["multisigs"][0].walletcreatefundedpsbt(inputs=[], outputs={to: value}, options={"feeRate": 0.00010}) + psbt = participants["multisigs"][0].walletcreatefundedpsbt(inputs=[], outputs={to: value}, feeRate=0.00010) for m in range(self.M): signers_multisig = participants["multisigs"][m] self._check_psbt(psbt["psbt"], to, value, signers_multisig) diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index b5d187076518..c12955647257 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -34,7 +34,7 @@ def run_test(self): self.log.info("Create a new transaction and wait until it's broadcast") parent_utxo, indep_utxo = node.listunspent()[:2] addr = node.getnewaddress() - txid = node.send(outputs=[{addr: 1}], options={"inputs": [parent_utxo]})["txid"] + txid = node.send(outputs=[{addr: 1}], inputs=[parent_utxo])["txid"] # Can take a few seconds due to transaction trickling def wait_p2p(): @@ -87,7 +87,7 @@ def wait_p2p(): # ordering of mapWallet is. mapWallet hashes txids with a salt drawn at random every time # the wallet is loaded, so reloading the wallet reshuffles its iteration order while # leaving the transactions themselves untouched. Reload until the child comes first. - child_txid = node.send(outputs=[{addr: 0.5}], options={"inputs": [{"txid":txid, "vout":0}]})["txid"] + child_txid = node.send(outputs=[{addr: 0.5}], inputs=[{"txid":txid, "vout":0}])["txid"] while True: txids = node.listreceivedbyaddress(minconf=0, address_filter=addr)[0]["txids"] if txids == [child_txid, txid]: @@ -111,7 +111,7 @@ def wait_p2p(): node.mockscheduler(60) # Evict these txs from the mempool - indep_send = node.send(outputs=[{node.getnewaddress(): 1}], options={"inputs": [indep_utxo]}) + indep_send = node.send(outputs=[{node.getnewaddress(): 1}], inputs=[indep_utxo]) node.getmempoolentry(indep_send["txid"]) assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, txid) assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, child_txid) diff --git a/test/functional/wallet_sendall.py b/test/functional/wallet_sendall.py index a57a69ba039d..bf11e6620d11 100755 --- a/test/functional/wallet_sendall.py +++ b/test/functional/wallet_sendall.py @@ -151,7 +151,7 @@ def sendall_invalid_amounts(self): self.log.info("Test sending more than balance") pre_sendall_balance = self.add_utxos([7, 14]) - expected_tx = self.wallet.sendall(recipients=[{self.recipient: 5}, self.remainder_target], options={"add_to_wallet": False}) + expected_tx = self.wallet.sendall(recipients=[{self.recipient: 5}, self.remainder_target], add_to_wallet=False) tx = self.wallet.decoderawtransaction(expected_tx['hex']) fee = 21 - sum([o["value"] for o in tx["vout"]]) @@ -192,7 +192,7 @@ def sendall_with_send_max(self): self.add_utxos([0.00000700, 0.00000600, 1]) # sendall with send_max - sendall_tx_receipt = self.wallet.sendall(recipients=[self.remainder_target], fee_rate=300, options={"send_max": True}) + sendall_tx_receipt = self.wallet.sendall(recipients=[self.remainder_target], fee_rate=300, send_max=True) tx_from_wallet = self.wallet.gettransaction(txid = sendall_tx_receipt["txid"], verbose = True) assert_equal(len(tx_from_wallet["decoded"]["vin"]), 1) @@ -208,7 +208,7 @@ def sendall_specific_inputs(self): self.add_utxos([17, 4]) utxo = self.wallet.listunspent()[0] - sendall_tx_receipt = self.wallet.sendall(recipients=[self.remainder_target], options={"inputs": [utxo]}) + sendall_tx_receipt = self.wallet.sendall(recipients=[self.remainder_target], inputs=[utxo]) tx_from_wallet = self.wallet.gettransaction(txid = sendall_tx_receipt["txid"], verbose = True) assert_equal(len(tx_from_wallet["decoded"]["vin"]), 1) assert_equal(len(tx_from_wallet["decoded"]["vout"]), 1) @@ -229,26 +229,26 @@ def sendall_fails_on_missing_input(self): # fails on out of bounds vout assert_raises_rpc_error(-8, "Input not found. UTXO ({}:{}) is not part of wallet.".format(spent_utxo["txid"], 1000), - self.wallet.sendall, recipients=[self.remainder_target], options={"inputs": [{"txid": spent_utxo["txid"], "vout": 1000}]}) + self.wallet.sendall, recipients=[self.remainder_target], inputs=[{"txid": spent_utxo["txid"], "vout": 1000}]) # fails on unconfirmed spent UTXO self.wallet.sendall(recipients=[self.remainder_target]) assert_raises_rpc_error(-8, "Input not available. UTXO ({}:{}) was already spent.".format(spent_utxo["txid"], spent_utxo["vout"]), - self.wallet.sendall, recipients=[self.remainder_target], options={"inputs": [spent_utxo]}) + self.wallet.sendall, recipients=[self.remainder_target], inputs=[spent_utxo]) # fails on specific previously spent UTXO, while other UTXOs exist self.generate(self.nodes[0], 1) self.add_utxos([19, 2]) assert_raises_rpc_error(-8, "Input not available. UTXO ({}:{}) was already spent.".format(spent_utxo["txid"], spent_utxo["vout"]), - self.wallet.sendall, recipients=[self.remainder_target], options={"inputs": [spent_utxo]}) + self.wallet.sendall, recipients=[self.remainder_target], inputs=[spent_utxo]) # fails because UTXO is unknown, while other UTXOs exist foreign_utxo = self.def_wallet.listunspent()[0] assert_raises_rpc_error(-8, "Input not found. UTXO ({}:{}) is not part of wallet.".format(foreign_utxo["txid"], foreign_utxo["vout"]), self.wallet.sendall, recipients=[self.remainder_target], - options={"inputs": [foreign_utxo]}) + inputs=[foreign_utxo]) @cleanup def sendall_fails_on_no_address(self): @@ -272,7 +272,7 @@ def sendall_fails_on_specific_inputs_with_send_max(self): "Cannot combine send_max with specific inputs.", self.wallet.sendall, recipients=[self.remainder_target], - options={"inputs": [utxo], "send_max": True}) + inputs=[utxo], send_max=True) @cleanup def sendall_fails_on_high_fee(self): @@ -310,7 +310,7 @@ def sendall_watchonly_specific_inputs(self): else: watchonly.importmulti(import_req) - sendall_tx_receipt = watchonly.sendall(recipients=[self.remainder_target], options={"inputs": [utxo]}) + sendall_tx_receipt = watchonly.sendall(recipients=[self.remainder_target], inputs=[utxo]) psbt = sendall_tx_receipt["psbt"] decoded = self.nodes[0].decodepsbt(psbt) assert_equal(len(decoded["inputs"]), 1) diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 907127eb650c..411f65e0c945 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -197,13 +197,13 @@ def test_valid_signer(self): self.log.info('Test send using hww1') # Don't broadcast transaction yet so the RPC returns the raw hex - res = hww.send(outputs={dest:0.5},options={"add_to_wallet": False}) + res = hww.send(outputs={dest:0.5},add_to_wallet=False) assert res["complete"] assert_equal(res["hex"], mock_tx) self.log.info('Test sendall using hww1') - res = hww.sendall(recipients=[{dest:0.5}, hww.getrawchangeaddress()],options={"add_to_wallet": False}) + res = hww.sendall(recipients=[{dest:0.5}, hww.getrawchangeaddress()], add_to_wallet=False) assert res["complete"] assert_equal(res["hex"], mock_tx) diff --git a/test/functional/wallet_timelock.py b/test/functional/wallet_timelock.py index 57a7c3907ade..0a622979a49c 100755 --- a/test/functional/wallet_timelock.py +++ b/test/functional/wallet_timelock.py @@ -29,7 +29,7 @@ def run_test(self): self.log.info("Send to new address with locktime") node.send( outputs={address: 5}, - options={"locktime": mtp_tip - 1}, + locktime=mtp_tip - 1, ) self.generate(node, 1) diff --git a/test/functional/wallet_watchonly.py b/test/functional/wallet_watchonly.py index 0d01a5714d26..0ac4b234d551 100755 --- a/test/functional/wallet_watchonly.py +++ b/test/functional/wallet_watchonly.py @@ -99,13 +99,13 @@ def run_test(self): options = {'changeAddress': wo_change} no_wo_options = {'changeAddress': wo_change, 'includeWatching': False} - result = wo_wallet.walletcreatefundedpsbt(inputs=inputs, outputs=outputs, options=options) + result = wo_wallet.walletcreatefundedpsbt(inputs=inputs, outputs=outputs, **options) assert_equal("psbt" in result, True) assert_raises_rpc_error(-4, "Insufficient funds", wo_wallet.walletcreatefundedpsbt, inputs, outputs, 0, no_wo_options) self.log.info('Testing fundrawtransaction watch-only defaults') rawtx = wo_wallet.createrawtransaction(inputs=inputs, outputs=outputs) - result = wo_wallet.fundrawtransaction(hexstring=rawtx, options=options) + result = wo_wallet.fundrawtransaction(hexstring=rawtx, **options) assert_equal("hex" in result, True) assert_raises_rpc_error(-4, "Insufficient funds", wo_wallet.fundrawtransaction, rawtx, no_wo_options) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index 4bda3dd7cbc0..7ea4998b7813 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -96,8 +96,8 @@ def main(): configfile = os.path.abspath(os.path.dirname(__file__)) + "/../config.ini" config.read_file(open(configfile, encoding="utf8")) - if not config["components"].getboolean("ENABLE_FUZZ"): - logging.error("Must have fuzz targets built") + if not config["components"].getboolean("ENABLE_FUZZ_BINARY"): + logging.error("Must have fuzz executable built") sys.exit(1) fuzz_bin=os.getenv("DASHFUZZ", default=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz')) @@ -154,11 +154,12 @@ def main(): ], env=get_fuzz_env(target=test_list_selection[0], source_dir=config['environment']['SRCDIR']), timeout=20, - check=True, + check=False, stderr=subprocess.PIPE, text=True, ).stderr - if "libFuzzer" not in help_output: + using_libfuzzer = "libFuzzer" in help_output + if (args.generate or args.m_dir) and not using_libfuzzer: logging.error("Must be built with libFuzzer") sys.exit(1) except subprocess.TimeoutExpired: @@ -192,6 +193,7 @@ def main(): test_list=test_list_selection, src_dir=config['environment']['SRCDIR'], fuzz_bin=fuzz_bin, + using_libfuzzer=using_libfuzzer, use_valgrind=args.valgrind, empty_min_time=args.empty_min_time, ) @@ -324,7 +326,7 @@ def job(t, args): future.result() -def run_once(*, fuzz_pool, corpus, test_list, src_dir, fuzz_bin, use_valgrind, empty_min_time): +def run_once(*, fuzz_pool, corpus, test_list, src_dir, fuzz_bin, using_libfuzzer, use_valgrind, empty_min_time): jobs = [] for t in test_list: corpus_path = corpus / t @@ -333,13 +335,16 @@ def run_once(*, fuzz_pool, corpus, test_list, src_dir, fuzz_bin, use_valgrind, e fuzz_bin, ] empty_dir = not any(corpus_path.iterdir()) - if empty_min_time and empty_dir: - args += [f"-max_total_time={empty_min_time}"] + if using_libfuzzer: + if empty_min_time and empty_dir: + args += [f"-max_total_time={empty_min_time}"] + else: + args += [ + "-runs=1", + corpus_path, + ] else: - args += [ - "-runs=1", - corpus_path, - ] + args += [corpus_path] if use_valgrind: args = ['valgrind', '--quiet', '--error-exitcode=1'] + args @@ -366,14 +371,7 @@ def job(t, args): logging.info(e.stdout) if e.stderr: logging.info(e.stderr) - logging.info("Target \"{}\" failed with exit code {}: {}".format(t, e.returncode, " ".join(args))) - sys.exit(1) - except subprocess.CalledProcessError as e: - if e.stdout: - logging.info(e.stdout) - if e.stderr: - logging.info(e.stderr) - logging.info("Target \"{}\" failed with exit code {}".format(" ".join(result.args), e.returncode)) + logging.info(f"Target {result.args} failed with exit code {e.returncode}") sys.exit(1) diff --git a/test/lint/lint-assertions.py b/test/lint/lint-assertions.py index 76e16147064b..d9f86b22b8f7 100755 --- a/test/lint/lint-assertions.py +++ b/test/lint/lint-assertions.py @@ -35,6 +35,16 @@ def main(): ":(exclude)src/rpc/server.cpp", ], "CHECK_NONFATAL(condition) or NONFATAL_UNREACHABLE should be used instead of assert for RPC code.") + # The `BOOST_ASSERT` macro requires to `#include boost/assert.hpp`, + # which is an unnecessary Boost dependency. + exit_code |= git_grep([ + "-E", + r"BOOST_ASSERT *\(.*\);", + "--", + "*.cpp", + "*.h", + ], "BOOST_ASSERT must be replaced with Assert, BOOST_REQUIRE, or BOOST_CHECK.") + sys.exit(exit_code) diff --git a/test/lint/lint-includes.py b/test/lint/lint-includes.py index 9cf5e2d99c8a..b09d8993e99f 100755 --- a/test/lint/lint-includes.py +++ b/test/lint/lint-includes.py @@ -31,7 +31,9 @@ "boost/signals2/optional_last_value.hpp", "boost/signals2/signal.hpp", "boost/test/included/unit_test.hpp", - "boost/test/unit_test.hpp"] + "boost/test/unit_test.hpp", + "boost/tuple/tuple.hpp", + ] EXCLUDED_BOOST_DIRS = ["src/immer/"]