diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 9f1a75a553..e53755c046 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -48,7 +48,10 @@ static void CoinSelection(benchmark::Bench& bench) coins.emplace_back(wtx.get(), 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */); } const CoinEligibilityFilter filter_standard(1, 6, 0); - const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 0, false); + const CoinSelectionParams coin_selection_params(/* use_bnb= */ true, /* change_output_size= */ 34, + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); bench.run([&] { std::set setCoinsRet; CAmount nValueRet; diff --git a/src/coinjoin/util.cpp b/src/coinjoin/util.cpp index 50a8e6727d..f337473c60 100644 --- a/src/coinjoin/util.cpp +++ b/src/coinjoin/util.cpp @@ -116,7 +116,7 @@ CTransactionBuilder::CTransactionBuilder(std::shared_ptr pwalletIn, con // Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not coinControl.m_discard_feerate = ::GetDiscardRate(*pwallet); // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction - coinControl.m_feerate = std::max(pwallet->chain().estimateSmartFee(int(pwallet->m_confirm_target), true, nullptr), pwallet->m_pay_tx_fee); + coinControl.m_feerate = std::max(GetRequiredFeeRate(*pwallet), pwallet->m_pay_tx_fee); // Change always goes back to origin coinControl.destChange = tallyItemIn.txdest; // Only allow tallyItems inputs for tx creation diff --git a/src/policy/feerate.h b/src/policy/feerate.h index e4bed40fd8..67b124d797 100644 --- a/src/policy/feerate.h +++ b/src/policy/feerate.h @@ -19,8 +19,8 @@ enum class FeeEstimateMode { UNSET, //!< Use default settings based on other criteria ECONOMICAL, //!< Force estimateSmartFee to use non-conservative estimates CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates - DASH_KB, //!< Use explicit DASH/kB fee given in coin control - DUFF_B, //!< Use explicit duff/B fee given in coin control + DASH_KB, //!< Use DASH/kB fee rate unit + DUFF_B, //!< Use duff/B fee rate unit }; /** @@ -39,7 +39,12 @@ public: // We've previously had bugs creep in from silent double->int conversion... static_assert(std::is_integral::value, "CFeeRate should be used without floats"); } - /** Constructor for a fee rate in satoshis per kB. The size in bytes must not exceed (2^63 - 1)*/ + /** Constructor for a fee rate in satoshis per kB (duff/kB). + * + * Passing a num_bytes value of COIN (1e8) returns a fee rate in satoshis per B (sat/B), + * e.g. (nFeePaid * 1e8 / 1e3) == (nFeePaid / 1e5), + * where 1e5 is the ratio to convert from DASH/kB to sat/B. + */ CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes); /** * Return the fee in satoshis for the given size in bytes. diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 8af1bcf441..86b7c2f950 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -44,7 +44,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendtoaddress", 6, "use_cj" }, { "sendtoaddress", 7, "conf_target" }, { "sendtoaddress", 9, "avoid_reuse" }, - { "sendtoaddress", 10, "verbose"}, + { "sendtoaddress", 10, "fee_rate"}, + { "sendtoaddress", 11, "verbose"}, { "settxfee", 0, "amount" }, { "sethdseed", 0, "newkeypool" }, { "getreceivedbyaddress", 1, "minconf" }, @@ -91,7 +92,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendmany", 6, "use_is" }, { "sendmany", 7, "use_cj" }, { "sendmany", 8, "conf_target" }, - { "sendmany", 10, "verbose" }, + { "sendmany", 10, "fee_rate" }, + { "sendmany", 11, "verbose" }, { "deriveaddresses", 1, "range" }, { "scantxoutset", 1, "scanobjects" }, { "addmultisigaddress", 0, "nrequired" }, @@ -152,7 +154,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "lockunspent", 1, "transactions" }, { "send", 0, "outputs" }, { "send", 1, "conf_target" }, - { "send", 3, "options" }, + { "send", 3, "fee_rate"}, + { "send", 4, "options" }, { "importprivkey", 2, "rescan" }, { "importelectrumwallet", 1, "index" }, { "importaddress", 2, "rescan" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 6b7e46dc33..e29eab9d8b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1160,7 +1160,7 @@ static RPCHelpMan estimatesmartfee() if (!request.params[1].isNull()) { FeeEstimateMode fee_mode; if (!FeeModeFromString(request.params[1].get_str(), fee_mode)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter"); + throw JSONRPCError(RPC_INVALID_PARAMETER, InvalidEstimateModeErrorMessage()); } if (fee_mode == FeeEstimateMode::ECONOMICAL) conservative = false; } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 3741a1b9a9..40d570830d 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -72,12 +72,12 @@ void RPCTypeCheckObj(const UniValue& o, } } -CAmount AmountFromValue(const UniValue& value) +CAmount AmountFromValue(const UniValue& value, int decimals) { if (!value.isNum() && !value.isStr()) throw JSONRPCError(RPC_TYPE_ERROR, "Amount is not a number or string"); CAmount amount; - if (!ParseFixedPoint(value.getValStr(), 8, &amount)) + if (!ParseFixedPoint(value.getValStr(), decimals, &amount)) throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount"); if (!MoneyRange(amount)) throw JSONRPCError(RPC_TYPE_ERROR, "Amount out of range"); @@ -319,11 +319,12 @@ UniValue DescribeAddress(const CTxDestination& dest) unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target) { - int target = value.get_int(); - if (target < 1 || (unsigned int)target > max_target) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid conf_target, must be between %u - %u", 1, max_target)); + const int target{value.get_int()}; + const unsigned int unsigned_target{static_cast(target)}; + if (target < 1 || unsigned_target > max_target) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid conf_target, must be between %u and %u", 1, max_target)); } - return (unsigned int)target; + return unsigned_target; } /** diff --git a/src/rpc/util.h b/src/rpc/util.h index 189551cd03..b39c7acf8e 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -84,7 +84,14 @@ int64_t ParseInt64V(const UniValue& v, const std::string &strName); double ParseDoubleV(const UniValue& v, const std::string &strName); bool ParseBoolV(const UniValue& v, const std::string &strName); -CAmount AmountFromValue(const UniValue& value); +/** + * Validate and return a CAmount from a UniValue number or string. + * + * @param[in] value UniValue number or string to parse. + * @param[in] decimals Number of significant digits (default: 8). + * @returns a CAmount if the various checks pass. + */ +CAmount AmountFromValue(const UniValue& value, int decimals = 8); using RPCArgList = std::vector>; std::string HelpExampleCli(const std::string& methodname, const std::string& args); diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp index 34d89ef7bd..efcc372d5b 100644 --- a/src/test/amount_tests.cpp +++ b/src/test/amount_tests.cpp @@ -109,6 +109,8 @@ BOOST_AUTO_TEST_CASE(ToStringTest) CFeeRate feeRate; feeRate = CFeeRate(1); BOOST_CHECK_EQUAL(feeRate.ToString(), "0.00000001 DASH/kB"); + BOOST_CHECK_EQUAL(feeRate.ToString(FeeEstimateMode::DASH_KB), "0.00000001 DASH/kB"); + BOOST_CHECK_EQUAL(feeRate.ToString(FeeEstimateMode::DUFF_B), "0.001 duff/B"); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index a796888a87..bc6269f03c 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -2044,6 +2044,15 @@ BOOST_AUTO_TEST_CASE(test_ParseFixedPoint) BOOST_CHECK(!ParseFixedPoint("1.1e", 8, &amount)); BOOST_CHECK(!ParseFixedPoint("1.1e-", 8, &amount)); BOOST_CHECK(!ParseFixedPoint("1.", 8, &amount)); + + // Test with 3 decimal places for fee rates in sat/vB. + BOOST_CHECK(ParseFixedPoint("0.001", 3, &amount)); + BOOST_CHECK_EQUAL(amount, CAmount{1}); + BOOST_CHECK(!ParseFixedPoint("0.0009", 3, &amount)); + BOOST_CHECK(!ParseFixedPoint("31.00100001", 3, &amount)); + BOOST_CHECK(!ParseFixedPoint("31.0011", 3, &amount)); + BOOST_CHECK(!ParseFixedPoint("31.99999999", 3, &amount)); + BOOST_CHECK(!ParseFixedPoint("31.999999999999999999999", 3, &amount)); } static void TestOtherThread(fs::path dirname, std::string lockname, bool *result) diff --git a/src/util/fees.cpp b/src/util/fees.cpp index 48df56be42..cbefe18dbb 100644 --- a/src/util/fees.cpp +++ b/src/util/fees.cpp @@ -40,8 +40,6 @@ const std::vector>& FeeModeMap() {"unset", FeeEstimateMode::UNSET}, {"economical", FeeEstimateMode::ECONOMICAL}, {"conservative", FeeEstimateMode::CONSERVATIVE}, - {(CURRENCY_UNIT + "/kB"), FeeEstimateMode::DASH_KB}, - {(CURRENCY_ATOM + "/B"), FeeEstimateMode::DUFF_B}, }; return FEE_MODES; } @@ -51,6 +49,11 @@ std::string FeeModes(const std::string& delimiter) return Join(FeeModeMap(), delimiter, [&](const std::pair& i) { return i.first; }); } +const std::string InvalidEstimateModeErrorMessage() +{ + return "Invalid estimate_mode parameter, must be one of: \"" + FeeModes("\", \"") + "\""; +} + bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode) { auto searchkey = ToUpper(mode_string); diff --git a/src/util/fees.h b/src/util/fees.h index 1ae8b8e322..9ef2389d3e 100644 --- a/src/util/fees.h +++ b/src/util/fees.h @@ -13,5 +13,6 @@ enum class FeeReason; bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode); std::string StringForFeeReason(FeeReason reason); std::string FeeModes(const std::string& delimiter); +const std::string InvalidEstimateModeErrorMessage(); #endif // BITCOIN_UTIL_FEES_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index fdea130b65..b308579e41 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -52,8 +51,6 @@ using interfaces::FoundBlock; static const std::string WALLET_ENDPOINT_BASE = "/wallet/"; -static const uint32_t WALLET_DASH_KB_TO_DUFF_B = COIN / 1000; // 1 duff / B = 0.00001 DASH / kB - static inline bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param) { bool can_avoid_reuse = wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); bool avoid_reuse = param.isNull() ? can_avoid_reuse : param.get_bool(); @@ -205,34 +202,35 @@ static std::string LabelFromValue(const UniValue& value) /** * Update coin control with fee estimation based on the given parameters * - * @param[in] wallet Wallet reference - * @param[in,out] cc Coin control which is to be updated - * @param[in] estimate_mode String value (e.g. "ECONOMICAL") - * @param[in] estimate_param Parameter (blocks to confirm, explicit fee rate, etc) - * @throws a JSONRPCError if estimate_mode is unknown, or if estimate_param is missing when required + * @param[in] wallet Wallet reference + * @param[in,out] cc Coin control to be updated + * @param[in] conf_target UniValue integer; confirmation target in blocks, values between 1 and 1008 are valid per policy/fees.h; + * @param[in] estimate_mode UniValue string; fee estimation mode, valid values are "unset", "economical" or "conservative"; + * @param[in] fee_rate UniValue real; fee rate in sat/B; + * if present, both conf_target and estimate_mode must either be null, or no-op or "unset" + * @param[in] override_min_fee bool; whether to set fOverrideFeeRate to true to disable minimum fee rate checks and instead + * verify only that fee_rate is greater than 0 + * @throws a JSONRPCError if conf_target, estimate_mode, or fee_rate contain invalid values or are in conflict */ -static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const UniValue& estimate_mode, const UniValue& estimate_param) +static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const UniValue& conf_target, const UniValue& estimate_mode, const UniValue& fee_rate, bool override_min_fee) { - if (!estimate_mode.isNull()) { - if (!FeeModeFromString(estimate_mode.get_str(), cc.m_fee_mode)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter"); + if (!fee_rate.isNull()) { + if (!conf_target.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate."); } + if (!estimate_mode.isNull() && estimate_mode.get_str() != "unset") { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate"); + } + // Fee rates in sat/vB cannot represent more than 3 significant digits. + cc.m_feerate = CFeeRate{AmountFromValue(fee_rate, /* decimals */ 3)}; + if (override_min_fee) cc.fOverrideFeeRate = true; + return; } - - if (cc.m_fee_mode == FeeEstimateMode::DASH_KB || cc.m_fee_mode == FeeEstimateMode::DUFF_B) { - if (estimate_param.isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Selected estimate_mode requires a fee rate"); - } - - CAmount fee_rate = AmountFromValue(estimate_param); - if (cc.m_fee_mode == FeeEstimateMode::DUFF_B) { - fee_rate /= WALLET_DASH_KB_TO_DUFF_B; - } - - cc.m_feerate = CFeeRate(fee_rate); - - } else if (!estimate_param.isNull()) { - cc.m_confirm_target = ParseConfirmTarget(estimate_param, wallet.chain().estimateMaxBlocks()); + if (!estimate_mode.isNull() && !FeeModeFromString(estimate_mode.get_str(), cc.m_fee_mode)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, InvalidEstimateModeErrorMessage()); + } + if (!conf_target.isNull()) { + cc.m_confirm_target = ParseConfirmTarget(conf_target, wallet.chain().estimateMaxBlocks()); } } @@ -385,6 +383,12 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto { EnsureWalletIsUnlocked(wallet); + // This function is only used by sendtoaddress and sendmany. + // This should always try to sign, if we don't have private keys, don't try to do anything here. + if (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); + } + if (coin_control.IsUsingCoinJoin()) { map_value["DS"] = "1"; } @@ -395,7 +399,7 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto bilingual_str error; CTransactionRef tx; FeeCalculation fee_calc_out; - bool fCreated = wallet.CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, !wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); + const bool fCreated = wallet.CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, true); if (!fCreated) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); } @@ -426,11 +430,12 @@ static RPCHelpMan sendtoaddress() "The recipient will receive less amount of Dash than you enter in the amount field."}, {"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"}, {"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Avoid spending from dirty addresses; addresses are considered\n" " dirty if they have previously been used in a transaction."}, + {"fee_rate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra information about the transaction."}, }, { @@ -444,15 +449,20 @@ static RPCHelpMan sendtoaddress() {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} }, }, - }, - RPCExamples{ - HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1") - + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"donation\" \"seans outpost\"") - + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"\" \"\" true") - + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"\" \"\" false true 0.00002 " + (CURRENCY_UNIT + "/kB")) - + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"\" \"\" false true 2 " + (CURRENCY_ATOM + "/B")) - + HelpExampleRpc("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\", 0.1, \"donation\", \"seans outpost\"") - }, + }, + RPCExamples{ + "\nSend 0.1 Dash\n" + + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1") + + "\nSend 0.1 Dash with a confirmation target of 6 blocks in economical fee estimate mode using positional arguments\n" + + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"donation\" \"sean's outpost\" false false false 6 economical") + + "\nSend 0.1 Dash with a fee rate of 1.1 " + CURRENCY_ATOM + "/B, subtract fee from amount, use CoinJoin funds only, using positional arguments\n" + + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"drinks\" \"room77\" true false true 0 \"\" 1.1") + + "\nSend 0.2 Dash with a confirmation target of 6 blocks in economical fee estimate mode using named arguments\n" + + HelpExampleCli("-named sendtoaddress", "address=\"" + EXAMPLE_ADDRESS[0] + "\" amount=0.2 conf_target=6 estimate_mode=\"economical\"") + + "\nSend 0.5 Dash with a fee rate of 25 " + CURRENCY_ATOM + "/B using named arguments\n" + + HelpExampleCli("-named sendtoaddress", "address=\"" + EXAMPLE_ADDRESS[0] + "\" amount=0.5 fee_rate=25") + + HelpExampleCli("-named sendtoaddress", "address=\"" + EXAMPLE_ADDRESS[0] + "\" amount=0.5 fee_rate=25 subtractfeefromamount=false avoid_reuse=true comment=\"2 pizzas\" comment_to=\"jeremy\" verbose=true") + }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); @@ -486,7 +496,7 @@ static RPCHelpMan sendtoaddress() // We also enable partial spend avoidance if reuse avoidance is set. coin_control.m_avoid_partial_spends |= coin_control.m_avoid_address_reuse; - SetFeeEstimateMode(*pwallet, coin_control, request.params[8], request.params[7]); + SetFeeEstimateMode(*pwallet, coin_control, /* conf_target */ request.params[7], /* estimate_mode */ request.params[8], /* fee_rate */ request.params[10], /* override_min_fee */ false); EnsureWalletIsUnlocked(*pwallet); @@ -500,7 +510,7 @@ static RPCHelpMan sendtoaddress() std::vector recipients; ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); - bool verbose = request.params[10].isNull() ? false: request.params[10].get_bool(); + const bool verbose{request.params[11].isNull() ? false : request.params[11].get_bool()}; return SendMoney(*pwallet, coin_control, recipients, mapValue, verbose); }, @@ -921,9 +931,10 @@ static RPCHelpMan sendmany() }, {"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"}, {"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, + {"fee_rate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra infomration about the transaction."}, }, { @@ -979,11 +990,11 @@ static RPCHelpMan sendmany() coin_control.UseCoinJoin(request.params[7].get_bool()); } - SetFeeEstimateMode(*pwallet, coin_control, request.params[9], request.params[8]); + SetFeeEstimateMode(*pwallet, coin_control, /* conf_target */ request.params[8], /* estimate_mode */ request.params[9], /* fee_rate */ request.params[10], /* override_min_fee */ false); std::vector recipients; ParseRecipients(sendTo, subtractFeeFromAmount, recipients); - bool verbose = request.params[10].isNull() ? false : request.params[10].get_bool(); + const bool verbose{request.params[11].isNull() ? false : request.params[11].get_bool()}; return SendMoney(*pwallet, coin_control, recipients, std::move(mapValue), verbose); }, @@ -3378,7 +3389,7 @@ static RPCHelpMan listunspent() }; } -void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options, CCoinControl& coinControl) +void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, const UniValue& options, CCoinControl& coinControl, bool override_min_fee) { // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -3411,7 +3422,8 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, {"lockUnspents", UniValueType(UniValue::VBOOL)}, {"lock_unspents", UniValueType(UniValue::VBOOL)}, {"locktime", UniValueType(UniValue::VNUM)}, - {"feeRate", UniValueType()}, // will be checked below + {"fee_rate", UniValueType()}, // will be checked by AmountFromValue() in SetFeeEstimateMode() + {"feeRate", UniValueType()}, // will be checked by AmountFromValue() below {"psbt", UniValueType(UniValue::VBOOL)}, {"subtractFeeFromOutputs", UniValueType(UniValue::VARR)}, {"subtract_fee_from_outputs", UniValueType(UniValue::VARR)}, @@ -3451,8 +3463,11 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, } if (options.exists("feeRate")) { + if (options.exists("fee_rate")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both fee_rate (" + CURRENCY_ATOM + "/B) and feeRate (" + CURRENCY_UNIT + "/kB)"); + } if (options.exists("conf_target")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate."); } if (options.exists("estimate_mode")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and feeRate"); @@ -3464,8 +3479,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, if (options.exists("subtractFeeFromOutputs") || options.exists("subtract_fee_from_outputs") ) subtractFeeFromOutputs = (options.exists("subtract_fee_from_outputs") ? options["subtract_fee_from_outputs"] : options["subtractFeeFromOutputs"]).get_array(); - SetFeeEstimateMode(wallet, coinControl, options["estimate_mode"], options["conf_target"]); - + SetFeeEstimateMode(wallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee); } } else { // if options is null and not a bool @@ -3524,7 +3538,8 @@ static RPCHelpMan fundrawtransaction() "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."}, {"lockUnspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, - {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set: makes wallet determine the fee", "Set a specific fee rate in " + CURRENCY_UNIT + "/kB"}, + {"fee_rate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, + {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_UNIT + "/kB."}, {"subtractFeeFromOutputs", RPCArg::Type::ARR, /* default */ "empty array", "The integers.\n" "The fee will be equally deducted from the amount of each specified output.\n" "Those recipients will receive less Dash than you enter in their corresponding amount field.\n" @@ -3533,7 +3548,7 @@ static RPCHelpMan fundrawtransaction() {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, }, @@ -3575,7 +3590,7 @@ static RPCHelpMan fundrawtransaction() CCoinControl coin_control; // Automatically select (additional) coins. Can be overridden by options.add_inputs. coin_control.m_add_inputs = true; - FundTransaction(*pwallet, tx, fee, change_position, request.params[1], coin_control); + FundTransaction(*pwallet, tx, fee, change_position, request.params[1], coin_control, /* override_min_fee */ true); UniValue result(UniValue::VOBJ); result.pushKV("hex", EncodeHexTx(CTransaction(tx))); @@ -4177,9 +4192,10 @@ static RPCHelpMan send() }, }, }, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, + {"fee_rate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { {"add_inputs", RPCArg::Type::BOOL, /* default */ "false", "If inputs are specified, automatically include more if they are not enough."}, @@ -4189,9 +4205,10 @@ static RPCHelpMan send() {"add_to_wallet", RPCArg::Type::BOOL, /* default */ "true", "When false, returns a serialized transaction which will not be added to the wallet or broadcast"}, {"change_address", RPCArg::Type::STR_HEX, /* default */ "pool address", "The Dash address to receive the change"}, {"change_position", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, + {"fee_rate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, {"include_watching", RPCArg::Type::BOOL, /* default */ "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."}, @@ -4205,7 +4222,7 @@ static RPCHelpMan send() {"locktime", RPCArg::Type::NUM, /* default */ "0", "Raw locktime. Non-0 value also locktime-activates inputs"}, {"lock_unspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, {"psbt", RPCArg::Type::BOOL, /* default */ "automatic", "Always return a PSBT, implies add_to_wallet=false."}, - {"subtract_fee_from_outputs", RPCArg::Type::ARR, /* default */ "empty array", "A JSON array of integers.\n" + {"subtract_fee_from_outputs", RPCArg::Type::ARR, /* default */ "empty array", "Outputs to subtract the fee from, specified as integer indices.\n" "The fee will be equally deducted from the amount of each specified output.\n" "Those recipients will receive less funds than you enter in their corresponding amount field.\n" "If no outputs are specified here, the sender pays the fee.", @@ -4226,36 +4243,53 @@ static RPCHelpMan send() } }, RPCExamples{"" - "\nSend with a fee rate of 1 " + CURRENCY_ATOM + "/B\n" - + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.1}' 1 " + CURRENCY_ATOM + "/B\n") + - "\nCreate a transaction that should confirm the next block, with a specific input, and return result without adding to wallet or broadcasting to the network\n" + "\nSend 0.1 Dash with a confirmation target of 6 blocks in economical fee estimate mode\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.1}' 6 economical\n") + + "Send 0.2 Dash with a fee rate of 1.1 " + CURRENCY_ATOM + "/B using positional arguments\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' null \"unset\" 1.1\n") + + "Send 0.2 Dash with a fee rate of 1 " + CURRENCY_ATOM + "/B using the options argument\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' null \"unset\" null '{\"fee_rate\": 1}'\n") + + "Send 0.3 Dash with a fee rate of 25 " + CURRENCY_ATOM + "/B using named arguments\n" + + HelpExampleCli("-named send", "outputs='{\"" + EXAMPLE_ADDRESS[0] + "\": 0.3}' fee_rate=25\n") + + "Create a transaction that should confirm the next block, with a specific input, and return result without adding to wallet or broadcasting to the network\n" + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.1}' 1 economical '{\"add_to_wallet\": false, \"inputs\": [{\"txid\":\"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0\", \"vout\":1}]}'") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue -{ - RPCTypeCheck(request.params, { - UniValueType(), // ARR or OBJ, checked later - UniValue::VNUM, - UniValue::VSTR, - UniValue::VOBJ - }, true - ); + { + RPCTypeCheck(request.params, { + UniValueType(), // outputs (ARR or OBJ, checked later) + UniValue::VNUM, // conf_target + UniValue::VSTR, // estimate_mode + UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode() + UniValue::VOBJ, // options + }, true + ); std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; - UniValue options = request.params[3]; - if (options.exists("feeRate") || options.exists("fee_rate") || options.exists("estimate_mode") || options.exists("conf_target")) { + UniValue options{request.params[4].isNull() ? UniValue::VOBJ : request.params[4]}; + if (options.exists("estimate_mode") || options.exists("conf_target")) { if (!request.params[1].isNull() || !request.params[2].isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Use either conf_target and estimate_mode or the options dictionary to control fee rate"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Pass conf_target and estimate_mode either as arguments or in the options object, but not both"); } } else { options.pushKV("conf_target", request.params[1]); options.pushKV("estimate_mode", request.params[2]); } + if (options.exists("fee_rate")) { + if (!request.params[3].isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Pass the fee_rate either as an argument, or in the options object, but not both"); + } + } else { + options.pushKV("fee_rate", request.params[3]); + } if (!options["conf_target"].isNull() && (options["estimate_mode"].isNull() || (options["estimate_mode"].get_str() == "unset"))) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Specify estimate_mode"); } + if (options.exists("feeRate")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use fee_rate (" + CURRENCY_ATOM + "/B) instead of feeRate"); + } if (options.exists("changeAddress")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_address"); } @@ -4279,9 +4313,9 @@ static RPCHelpMan send() CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"]); CCoinControl coin_control; // Automatically select coins, unless at least one is manually selected. Can - // be overriden by options.add_inputs. + // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; - FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control); + FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control, /* override_min_fee */ false); bool add_to_wallet = true; if (options.exists("add_to_wallet")) { @@ -4525,6 +4559,7 @@ static RPCHelpMan walletcreatefundedpsbt() {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only"}, {"lockUnspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, + {"fee_rate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_ATOM + "/B."}, {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set: makes wallet determine the fee", "Set a specific fee rate in " + CURRENCY_UNIT + "/kB"}, {"subtractFeeFromOutputs", RPCArg::Type::ARR, /* default */ "empty array", "The outputs to subtract the fee from.\n" "The fee will be equally deducted from the amount of each specified output.\n" @@ -4534,7 +4569,7 @@ static RPCHelpMan walletcreatefundedpsbt() {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, - {"conf_target", RPCArg::Type::NUM, /* default */ "Fallback to wallet's confirmation target", "Confirmation target (in blocks)"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, }, @@ -4579,7 +4614,7 @@ static RPCHelpMan walletcreatefundedpsbt() // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; - FundTransaction(*pwallet, rawTx, fee, change_position, request.params[3], coin_control); + FundTransaction(*pwallet, rawTx, fee, change_position, request.params[3], coin_control, /* override_min_fee */ true); // Make a blank psbt PartiallySignedTransaction psbtx{rawTx}; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 3ec2f7a094..88eb119d7b 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -33,9 +33,10 @@ static const CoinEligibilityFilter filter_standard(1, 6, 0); static const CoinEligibilityFilter filter_confirmed(1, 1, 0); static const CoinEligibilityFilter filter_standard_extra(6, 6, 0); -CoinSelectionParams coin_selection_params(/* use_bnb = */ false, /* change_output_size = */ 0, - /* change_spend_size = */ 0, /* effective_fee = */ CFeeRate(0), - /* tx_noinputs_size = */ 0, /* m_avoid_partial_spends = */ false); +CoinSelectionParams coin_selection_params(/* use_bnb= */ false, /* change_output_size= */ 0, + /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); static void add_coin(const CAmount& nValue, int nInput, std::vector& set) { @@ -254,9 +255,10 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) } // Make sure that effective value is working in SelectCoinsMinConf when BnB is used - CoinSelectionParams coin_selection_params_bnb(/* use_bnb = */ true, /* change_output_size = */ 0, - /* change_spend_size = */ 0, /* effective_fee = */ CFeeRate(3000), - /* tx_noinputs_size = */ 0, /* m_avoid_partial_spends = */ false); + CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 0, + /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(3000), + /* long_term_feerate= */ CFeeRate(1000), /* discard_feerate= */ CFeeRate(1000), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); @@ -298,7 +300,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) CCoinControl coin_control; coin_control.fAllowOtherInputs = true; coin_control.Select(COutPoint(coins.at(0).tx->GetHash(), coins.at(0).i)); - coin_selection_params_bnb.effective_fee = CFeeRate(0); + coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); BOOST_CHECK(wallet->SelectCoins(coins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used)); BOOST_CHECK(bnb_used); BOOST_CHECK(coin_selection_params_bnb.use_bnb); @@ -642,12 +644,14 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) CAmount target = rand.randrange(balance - 1000) + 1000; // Perform selection - CoinSelectionParams coin_selection_params_knapsack(/* use_bnb = */ false, /* change_output_size = */ 34, - /* change_spend_size = */ 148, /* effective_fee = */ CFeeRate(0), - /* tx_noinputs_size = */ 0, /* m_avoid_partial_spends = */ false); - CoinSelectionParams coin_selection_params_bnb(/* use_bnb = */ true, /* change_output_size = */ 34, - /* change_spend_size = */ 148, /* effective_fee = */ CFeeRate(0), - /* tx_noinputs_size = */ 0, /* m_avoid_partial_spends = */ false); + CoinSelectionParams coin_selection_params_knapsack(/* use_bnb= */ false, /* change_output_size= */ 34, + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); + CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 34, + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); CoinSet out_set; CAmount out_value = 0; bool bnb_used = false; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 14ba64998d..414eeb16aa 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2872,26 +2872,20 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil nValueRet = 0; if (coin_selection_params.use_bnb) { - // Get long term estimate - FeeCalculation feeCalc; - CCoinControl temp; - temp.m_confirm_target = 1008; - CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc); - // Get the feerate for effective value. // When subtracting the fee from the outputs, we want the effective feerate to be 0 CFeeRate effective_feerate{0}; if (!coin_selection_params.m_subtract_fee_outputs) { - effective_feerate = coin_selection_params.effective_fee; + effective_feerate = coin_selection_params.m_effective_feerate; } - std::vector groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, long_term_feerate, eligibility_filter, true /* positive_only */); + std::vector groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, true /* positive_only */); // Calculate cost of change - CAmount cost_of_change = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); + CAmount cost_of_change = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size) + coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.change_output_size); // Calculate the fees for things that aren't inputs - CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); + CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); bnb_used = true; return SelectCoinsBnB(groups, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); } else { @@ -2959,7 +2953,7 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm if (coin.m_input_bytes <= 0) { return false; // Not solvable, can't estimate size for fee } - coin.effective_value = coin.txout.nValue - coin_selection_params.effective_fee.GetFee(coin.m_input_bytes); + coin.effective_value = coin.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(coin.m_input_bytes); if (coin_selection_params.use_bnb) { value_to_select -= coin.effective_value; } else { @@ -3548,7 +3542,19 @@ bool CWallet::CreateTransactionInternal( CMutableTransaction txNew; FeeCalculation feeCalc; - CFeeRate discard_rate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(*this); + + CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy + coin_selection_params.m_discard_feerate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(*this); + + // Get the fee rate to use effective values in coin selection + coin_selection_params.m_effective_feerate = GetMinimumFeeRate(*this, coin_control, &feeCalc); + // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly + // provided one + if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) { + error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::DUFF_B), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::DUFF_B)); + return false; + } + int nBytes{0}; { std::vector vecCoins; @@ -3558,7 +3564,6 @@ bool CWallet::CreateTransactionInternal( CAmount nAmountAvailable{0}; std::vector vAvailableCoins; AvailableCoins(vAvailableCoins, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0); - CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy coin_selection_params.use_bnb = false; // never use BnB for (auto out : vAvailableCoins) { @@ -3746,7 +3751,7 @@ bool CWallet::CreateTransactionInternal( // Never create dust outputs; if we would, just // add the dust to the fee. - if (IsDust(newTxOut, discard_rate)) + if (IsDust(newTxOut, coin_selection_params.m_discard_feerate)) { nFeeRet = nFeePrev; nChangePosInOut = -1; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 194d734061..11824d79ee 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -683,8 +683,13 @@ struct CoinSelectionParams size_t change_output_size = 0; /** Size of the input to spend a change output in virtual bytes. */ size_t change_spend_size = 0; - /** The targeted feerate of the transaction being built. */ - CFeeRate effective_fee = CFeeRate(0); + /** The fee to spend these UTXOs at the long term feerate. */ + CFeeRate m_effective_feerate; + /** The feerate estimate used to estimate an upper bound on what should be sufficient to spend + * the change output sometime in the future. */ + CFeeRate m_long_term_feerate; + /** If the cost to spend a change output at the discard feerate exceeds its value, drop it to fees. */ + CFeeRate m_discard_feerate; size_t tx_noinputs_size = 0; /** Indicate that we are subtracting the fee from outputs */ bool m_subtract_fee_outputs = false; @@ -693,11 +698,14 @@ struct CoinSelectionParams * reuse. Dust outputs are not eligible to be added to output groups and thus not considered. */ bool m_avoid_partial_spends = false; - CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size, bool avoid_partial) : + CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_feerate, + CFeeRate long_term_feerate, CFeeRate discard_feerate, size_t tx_noinputs_size, bool avoid_partial) : use_bnb(use_bnb), change_output_size(change_output_size), change_spend_size(change_spend_size), - effective_fee(effective_fee), + m_effective_feerate(effective_feerate), + m_long_term_feerate(long_term_feerate), + m_discard_feerate(discard_feerate), tx_noinputs_size(tx_noinputs_size), m_avoid_partial_spends(avoid_partial) {} diff --git a/test/functional/rpc_estimatefee.py b/test/functional/rpc_estimatefee.py index a76d9eaf8e..51b7efb4c3 100755 --- a/test/functional/rpc_estimatefee.py +++ b/test/functional/rpc_estimatefee.py @@ -27,7 +27,7 @@ class EstimateFeeTest(BitcoinTestFramework): # wrong type for estimatesmartfee(estimate_mode) assert_raises_rpc_error(-3, "Expected type string, got number", self.nodes[0].estimatesmartfee, 1, 1) - assert_raises_rpc_error(-8, "Invalid estimate_mode parameter", self.nodes[0].estimatesmartfee, 1, 'foo') + assert_raises_rpc_error(-8, 'Invalid estimate_mode parameter, must be one of: "unset", "economical", "conservative"', self.nodes[0].estimatesmartfee, 1, 'foo') # wrong type for estimaterawfee(threshold) assert_raises_rpc_error(-3, "Expected type number, got string", self.nodes[0].estimaterawfee, 1, 'foo') diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index bf33d1092d..a1c1334e94 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -5,9 +5,12 @@ """Test the fundrawtransaction RPC.""" from decimal import Decimal +from itertools import product + from test_framework.descriptors import descsum_create from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( + assert_approx, assert_equal, assert_fee_amount, assert_greater_than, @@ -702,21 +705,92 @@ class RawTransactionsTest(BitcoinTestFramework): wwatch.unloadwallet() def test_option_feerate(self): - self.log.info("Test fundrawtxn feeRate option") - + self.log.info("Test fundrawtxn with explicit fee rates (fee_rate duff/B and feeRate DASH/kB)") + node = self.nodes[3] # Make sure there is exactly one input so coin selection can't skew the result. assert_equal(len(self.nodes[3].listunspent(1)), 1) - inputs = [] - outputs = {self.nodes[3].getnewaddress() : 1} - rawtx = self.nodes[3].createrawtransaction(inputs, outputs) - result = self.nodes[3].fundrawtransaction(rawtx) # uses self.min_relay_tx_fee (set by settxfee) - result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}) - result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10 * self.min_relay_tx_fee}) - assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[3].fundrawtransaction, rawtx, {"feeRate": 1}) + outputs = {node.getnewaddress() : 1} + rawtx = node.createrawtransaction(inputs, outputs) + + 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)}) + result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex']) + assert_fee_amount(result1['fee'], count_bytes(result1['hex']), 2 * result_fee_rate) assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate) assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate) + assert_fee_amount(result4['fee'], count_bytes(result4['hex']), 10 * result_fee_rate) + + # Test that funding non-standard "zero-fee" transactions is valid. + for param, zero_value in product(["fee_rate", "feeRate"], [0, 0.000, 0.00000000, "0", "0.000", "0.00000000"]): + assert_equal(self.nodes[3].fundrawtransaction(rawtx, {param: zero_value})["fee"], 0) + + # 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}) + 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, "Expected type string for estimate_mode, got {}".format(k), + 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}) + + 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, "Expected type number for conf_target, got {}".format(k), + 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}) + + 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}) + assert_raises_rpc_error(-3, "Amount out of range", + node.fundrawtransaction, rawtx, {param: -1, "add_inputs": True}) + assert_raises_rpc_error(-3, "Amount is not a number or string", + node.fundrawtransaction, rawtx, {param: {"foo": "bar"}, "add_inputs": True}) + # 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}) + # Test fee_rate values that cannot be represented in sat/vB. + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "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}) + + 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}) + + 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}) + + 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}) + + for param in ["feeRate", "fee_rate"]: + self.log.info("- raises RPC error if both {} and conf_target are passed".format(param)) + assert_raises_rpc_error(-8, "Cannot specify both conf_target and {}. Please provide either a confirmation " + "target in blocks for automatic fee estimation, or an explicit fee rate.".format(param), + node.fundrawtransaction, rawtx, {param: 1, "conf_target": 1, "add_inputs": True}) + + 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}) def test_address_reuse(self): """Test no address reuse occurs.""" @@ -744,12 +818,32 @@ class RawTransactionsTest(BitcoinTestFramework): outputs = {self.nodes[2].getnewaddress(): 1} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) + # 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]}),] + 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)] + assert_equal(result[0]['fee'], result[1]['fee'], result[2]['fee']) + assert_equal(result[3]['fee'], result[4]['fee']) + assert_equal(change[0], change[1]) + assert_equal(output[0], output[1]) + assert_equal(output[0], output[2] + result[2]['fee']) + assert_equal(change[0] + result[0]['fee'], change[2]) + assert_equal(output[3], output[4] + result[4]['fee']) + assert_equal(change[3] + result[3]['fee'], change[4]) + + # 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]}),] 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)] diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index cb5bd668a8..e05ccad04f 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -6,6 +6,8 @@ """ from decimal import Decimal +from itertools import product + from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_approx, @@ -94,8 +96,11 @@ class PSBTTest(BitcoinTestFramework): elif out['scriptPubKey']['address'] == p2pkh: p2pkh_pos = out['n'] + inputs = [{"txid":txid,"vout":p2pkh_pos}] + outputs = [{self.nodes[1].getnewaddress(): 9.99}] + # spend single key from node 1 - created_psbt = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}) + created_psbt = self.nodes[1].walletcreatefundedpsbt(inputs, outputs) walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(created_psbt['psbt']) # Make sure it has both types of UTXOs decoded = self.nodes[1].decodepsbt(walletprocesspsbt_out['psbt']) @@ -105,18 +110,83 @@ class PSBTTest(BitcoinTestFramework): assert_equal(walletprocesspsbt_out['complete'], True) self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex']) - # feeRate of 0.1 DASH / KB produces a total fee slightly below -maxtxfee (~0.06650000): - res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 0.1, "add_inputs": True}, False) - assert_approx(res["fee"], 0.04, 0.03) - decoded_psbt = self.nodes[0].decodepsbt(res['psbt']) - for psbt_in in decoded_psbt["inputs"]: - assert "bip32_derivs" not in psbt_in + self.log.info("Test walletcreatefundedpsbt fee rate of 10000 sat/vB and 0.1 BTC/kvB produces a total fee at or slightly below -maxtxfee (~0.05290000)") + res1 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": 10000, "add_inputs": True}) + assert_approx(res1["fee"], 0.04, 0.005) + res2 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": "0.1", "add_inputs": True}) + assert_approx(res2["fee"], 0.04, 0.005) - # feeRate of 10 DASH / KB produces a total fee well above -maxtxfee + self.log.info("Test min fee rate checks with walletcreatefundedpsbt are bypassed, e.g. a fee_rate under 1 sat/vB is allowed") + res3 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": "0.999", "add_inputs": True}) + assert_approx(res3["fee"], 0.00000224, 0.0000001) + res4 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.00000999, "add_inputs": True}) + assert_approx(res4["fee"], 0.00000224, 0.0000001) + + self.log.info("Test min fee rate checks with walletcreatefundedpsbt are bypassed and that funding non-standard 'zero-fee' transactions is valid") + for param, zero_value in product(["fee_rate", "feeRate"], [0, 0.000, 0.00000000, "0", "0.000", "0.00000000"]): + assert_equal(0, self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {param: zero_value, "add_inputs": True})["fee"]) + + self.log.info("Test invalid fee rate settings") + for param, value in {("fee_rate", 100000), ("feeRate", 1)}: + assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: value, "add_inputs": True}) + assert_raises_rpc_error(-3, "Amount out of range", + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: -1, "add_inputs": True}) + assert_raises_rpc_error(-3, "Amount is not a number or string", + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: {"foo": "bar"}, "add_inputs": True}) + # 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", + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: invalid_value, "add_inputs": True}) + # Test fee_rate values that cannot be represented in sat/vB. + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + assert_raises_rpc_error(-3, "Invalid amount", + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": invalid_value, "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)", + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"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", + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"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)) + assert_raises_rpc_error(-8, "Cannot specify both conf_target and {}. Please provide either a confirmation " + "target in blocks for automatic fee estimation, or an explicit fee rate.".format(param), + self.nodes[1].walletcreatefundedpsbt ,inputs, outputs, 0, {param: 1, "conf_target": 1, "add_inputs": True}) + + 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", + self.nodes[1].walletcreatefundedpsbt ,inputs, outputs, 0, {"fee_rate": 1, "estimate_mode": "economical", "add_inputs": True}) + + self.log.info("- raises RPC error with invalid estimate_mode settings") + for k, v in {"number": 42, "object": {"foo": "bar"}}.items(): + assert_raises_rpc_error(-3, "Expected type string for estimate_mode, got {}".format(k), + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"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"', + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"estimate_mode": mode, "conf_target": 0.1, "add_inputs": True}) + + self.log.info("- raises RPC error 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, "Expected type number for conf_target, got {}".format(k), + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"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 + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"estimate_mode": mode, "conf_target": n, "add_inputs": True}) + + self.log.info("Test walletcreatefundedpsbt with too-high fee rate produces total fee well above -maxtxfee and raises RPC error") # previously this was silently capped at -maxtxfee - assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 10, "add_inputs": True}) - assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():1}, 0, {"feeRate": 10, "add_inputs": True}) + for bool_add, outputs_array in {True: outputs, False: [{self.nodes[1].getnewaddress(): 1}]}.items(): + msg = "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)" + assert_raises_rpc_error(-4, msg, self.nodes[1].walletcreatefundedpsbt, inputs, outputs_array, 0, {"fee_rate": 1000000, "add_inputs": bool_add}) + assert_raises_rpc_error(-4, msg, self.nodes[1].walletcreatefundedpsbt, inputs, outputs_array, 0, {"feeRate": 1, "add_inputs": bool_add}) + 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'] walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(psbtx) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index ea2b49b994..f9d82ed4a8 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the wallet.""" from decimal import Decimal +from itertools import product from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework @@ -16,6 +17,9 @@ from test_framework.util import ( ) from test_framework.wallet_util import test_address +NOT_A_NUMBER_OR_STRING = "Amount is not a number or string" +OUT_OF_RANGE = "Amount out of range" + class WalletTest(BitcoinTestFramework): def set_test_params(self): @@ -50,6 +54,9 @@ class WalletTest(BitcoinTestFramework): assert_fee_amount(fee, tx_size, fee_per_byte * 1000) return curr_balance + def get_vsize(self, txn): + return self.nodes[0].decoderawtransaction(txn)['size'] + def run_test(self): # Check that there's no UTXO on none of the nodes @@ -79,7 +86,7 @@ class WalletTest(BitcoinTestFramework): assert_equal(len(self.nodes[1].listunspent()), 1) assert_equal(len(self.nodes[2].listunspent()), 0) - self.log.info("test gettxout") + self.log.info("Test gettxout") confirmed_txid, confirmed_index = utxos[0]["txid"], utxos[0]["vout"] # First, outputs that are unspent both in the chain and in the # mempool should appear with or without include_mempool @@ -93,7 +100,7 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 110) mempool_txid = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 100) - self.log.info("test gettxout (second part)") + self.log.info("Test gettxout (second part)") # utxo spent in mempool should be visible if you exclude mempool # but invisible if you include mempool txout = self.nodes[0].gettxout(confirmed_txid, confirmed_index, False) @@ -221,6 +228,8 @@ class WalletTest(BitcoinTestFramework): assert_equal(self.nodes[2].getbalance(), node_2_bal) node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal('200'), fee_per_byte, count_bytes(self.nodes[2].gettransaction(txid)['hex'])) + self.log.info("Test sendmany") + # Sendmany 100 DASH txid = self.nodes[2].sendmany('', {address: 100}, 0, False, "", []) self.nodes[2].generate(1) @@ -237,62 +246,69 @@ class WalletTest(BitcoinTestFramework): assert_equal(self.nodes[2].getbalance(), node_2_bal) node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('100'), fee_per_byte, count_bytes(self.nodes[2].gettransaction(txid)['hex'])) - self.start_node(3, self.nodes[3].extra_args) - self.connect_nodes(0, 3) - # Sendmany with explicit fee (DASH/kB) - # Throw if no conf_target provided - assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate", - self.nodes[2].sendmany, - amounts={ address: 10 }, - estimate_mode='dash/kB') - # Throw if negative feerate - assert_raises_rpc_error(-3, "Amount out of range", - self.nodes[2].sendmany, - amounts={ address: 10 }, - conf_target=-1, - estimate_mode='dash/kB') - fee_per_kb = 0.0002500 - explicit_fee_per_byte = Decimal(fee_per_kb) / 1000 - txid = self.nodes[2].sendmany( - amounts={ address: 10 }, - conf_target=fee_per_kb, - estimate_mode='dash/kB', - ) - self.nodes[2].generate(1) - self.sync_all(self.nodes[0:3]) - node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), node_2_bal - Decimal('10'), explicit_fee_per_byte, count_bytes(self.nodes[2].gettransaction(txid)['hex'])) - assert_equal(self.nodes[2].getbalance(), node_2_bal) - node_0_bal += Decimal('10') - assert_equal(self.nodes[0].getbalance(), node_0_bal) + self.log.info("Test sendmany with fee_rate param (explicit fee rate in duff/B)") + fee_rate_sat_vb = 2 + fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 + explicit_fee_rate_btc_kvb = Decimal(fee_rate_btc_kvb) / 1000 - # Sendmany with explicit fee (DUFF/B) - # Throw if no conf_target provided - assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate", - self.nodes[2].sendmany, - amounts={ address: 10 }, - estimate_mode='duff/b') - # Throw if negative feerate - assert_raises_rpc_error(-3, "Amount out of range", - self.nodes[2].sendmany, - amounts={ address: 10 }, - conf_target=-1, - estimate_mode='duff/b') - fee_duff_per_b = 2 - fee_per_kb = fee_duff_per_b / 100000.0 - explicit_fee_per_byte = Decimal(fee_per_kb) / 1000 - txid = self.nodes[2].sendmany( - amounts={ address: 10 }, - conf_target=fee_duff_per_b, - estimate_mode='duff/b', - ) + # Test passing fee_rate as a string + txid = self.nodes[2].sendmany(amounts={address: 10}, fee_rate=str(fee_rate_sat_vb)) self.nodes[2].generate(1) self.sync_all(self.nodes[0:3]) balance = self.nodes[2].getbalance() - node_2_bal = self.check_fee_amount(balance, node_2_bal - Decimal('10'), explicit_fee_per_byte, count_bytes(self.nodes[2].gettransaction(txid)['hex'])) + node_2_bal = self.check_fee_amount(balance, node_2_bal - Decimal('10'), explicit_fee_rate_btc_kvb, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) assert_equal(balance, node_2_bal) node_0_bal += Decimal('10') assert_equal(self.nodes[0].getbalance(), node_0_bal) + # Test passing fee_rate as an integer + amount = Decimal("0.0001") + txid = self.nodes[2].sendmany(amounts={address: amount}, fee_rate=fee_rate_sat_vb) + self.nodes[2].generate(1) + self.sync_all(self.nodes[0:3]) + balance = self.nodes[2].getbalance() + node_2_bal = self.check_fee_amount(balance, node_2_bal - amount, explicit_fee_rate_btc_kvb, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) + assert_equal(balance, node_2_bal) + node_0_bal += amount + assert_equal(self.nodes[0].getbalance(), node_0_bal) + + for key in ["totalFee", "feeRate"]: + assert_raises_rpc_error(-8, "Unknown named parameter key", self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=1, key=1) + + # Test setting explicit fee rate just below the minimum. + self.log.info("Test sendmany raises 'fee rate too low' if fee_rate of 0.99999999 is passed") + assert_raises_rpc_error(-6, "Fee rate (0.999 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)", + self.nodes[2].sendmany, amounts={address: 10}, fee_rate=0.999) + + self.log.info("Test sendmany raises if an invalid fee_rate is passed") + # Test fee_rate with zero values. + msg = "Fee rate (0.000 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)" + for zero_value in [0, 0.000, 0.00000000, "0", "0.000", "0.00000000"]: + assert_raises_rpc_error(-6, msg, self.nodes[2].sendmany, amounts={address: 1}, fee_rate=zero_value) + msg = "Invalid amount" + # 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, msg, self.nodes[2].sendmany, amounts={address: 1.0}, fee_rate=invalid_value) + # Test fee_rate values that cannot be represented in duff/B. + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + assert_raises_rpc_error(-3, msg, self.nodes[2].sendmany, amounts={address: 10}, fee_rate=invalid_value) + # Test fee_rate out of range (negative number). + assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[2].sendmany, amounts={address: 10}, fee_rate=-1) + # Test type error. + for invalid_value in [True, {"foo": "bar"}]: + assert_raises_rpc_error(-3, NOT_A_NUMBER_OR_STRING, self.nodes[2].sendmany, amounts={address: 10}, fee_rate=invalid_value) + + self.log.info("Test sendmany raises if an invalid conf_target or estimate_mode is passed") + for target, mode in product([-1, 0, 1009], ["economical", "conservative"]): + assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", # max value of 1008 per src/policy/fees.h + self.nodes[2].sendmany, amounts={address: 1}, conf_target=target, estimate_mode=mode) + for target, mode in product([-1, 0], ["btc/kb", "sat/b"]): + assert_raises_rpc_error(-8, 'Invalid estimate_mode parameter, must be one of: "unset", "economical", "conservative"', + self.nodes[2].sendmany, amounts={address: 1}, conf_target=target, estimate_mode=mode) + + self.start_node(3, self.nodes[3].extra_args) + self.connect_nodes(0, 3) + self.sync_all() # check if we can list zero value tx as available coins # 1. create raw_tx @@ -321,7 +337,7 @@ class WalletTest(BitcoinTestFramework): assert_equal(uTx['amount'], Decimal('0')) assert found - # do some -walletbroadcast tests + self.log.info("Test -walletbroadcast") self.stop_nodes() self.start_node(0, ["-walletbroadcast=0"]) self.start_node(1, ["-walletbroadcast=0"]) @@ -381,7 +397,7 @@ class WalletTest(BitcoinTestFramework): # General checks for errors from incorrect inputs # This will raise an exception because the amount is negative - assert_raises_rpc_error(-3, "Amount out of range", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "-1") + assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "-1") # This will raise an exception because the amount type is wrong assert_raises_rpc_error(-3, "Invalid amount", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "1f-4") @@ -423,73 +439,70 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].generate(1) self.sync_all(self.nodes[0:3]) - # send with explicit dash/kb fee - self.log.info("test explicit fee (sendtoaddress as dash/kb)") - self.nodes[0].generate(1) - self.sync_all(self.nodes[0:3]) + self.log.info("Test sendtoaddress with fee_rate param (explicit fee rate in duff/B)") prebalance = self.nodes[2].getbalance() assert prebalance > 2 address = self.nodes[1].getnewaddress() - # Throw if no conf_target provided - assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate", - self.nodes[2].sendtoaddress, - address=address, - amount=1.0, - estimate_mode='dash/Kb') - # Throw if negative feerate - assert_raises_rpc_error(-3, "Amount out of range", - self.nodes[2].sendtoaddress, - address=address, - amount=1.0, - conf_target=-1, - estimate_mode='dash/kb') - txid = self.nodes[2].sendtoaddress( - address=address, - amount=1.0, - conf_target=0.00002500, - estimate_mode='dash/kb', - ) - tx_size = count_bytes(self.nodes[2].gettransaction(txid)['hex']) - self.sync_all(self.nodes[0:3]) + amount = 3 + fee_rate_sat_vb = 2 + fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 + # Test passing fee_rate as an integer + txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=fee_rate_sat_vb) + tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex']) self.nodes[0].generate(1) self.sync_all(self.nodes[0:3]) postbalance = self.nodes[2].getbalance() - fee = prebalance - postbalance - Decimal('1') - assert_fee_amount(fee, tx_size, Decimal('0.00002500')) + fee = prebalance - postbalance - Decimal(amount) + assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb)) - # send with explicit duff/b fee - self.sync_all(self.nodes[0:3]) - self.log.info("test explicit fee (sendtoaddress as duff/b)") - self.nodes[0].generate(1) prebalance = self.nodes[2].getbalance() - assert prebalance > 2 - address = self.nodes[1].getnewaddress() - # Throw if no conf_target provided - assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate", - self.nodes[2].sendtoaddress, - address=address, - amount=1.0, - estimate_mode='duff/b') - # Throw if negative feerate - assert_raises_rpc_error(-3, "Amount out of range", - self.nodes[2].sendtoaddress, - address=address, - amount=1.0, - conf_target=-1, - estimate_mode='duff/b') - txid = self.nodes[2].sendtoaddress( - address=address, - amount=1.0, - conf_target=2, - estimate_mode='duff/B', - ) - tx_size = count_bytes(self.nodes[2].gettransaction(txid)['hex']) - self.sync_all(self.nodes[0:3]) + amount = Decimal("0.001") + fee_rate_sat_vb = 1.23 + fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 + # Test passing fee_rate as a string + txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=str(fee_rate_sat_vb)) + tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex']) self.nodes[0].generate(1) self.sync_all(self.nodes[0:3]) postbalance = self.nodes[2].getbalance() - fee = prebalance - postbalance - Decimal('1') - assert_fee_amount(fee, tx_size, Decimal('0.00002000')) + fee = prebalance - postbalance - amount + # TODO: remove workaround after bitcoin/bitcoin#22949 done + workaround_offset = 1 + assert_fee_amount(fee, tx_size - workaround_offset, Decimal(fee_rate_btc_kvb)) + + for key in ["totalFee", "feeRate"]: + assert_raises_rpc_error(-8, "Unknown named parameter key", self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=1, key=1) + + # Test setting explicit fee rate just below the minimum. + self.log.info("Test sendtoaddress raises 'fee rate too low' if fee_rate of 0.99999999 is passed") + assert_raises_rpc_error(-6, "Fee rate (0.999 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)", + self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=0.999) + + self.log.info("Test sendtoaddress raises if an invalid fee_rate is passed") + # Test fee_rate with zero values. + msg = "Fee rate (0.000 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)" + for zero_value in [0, 0.000, 0.00000000, "0", "0.000", "0.00000000"]: + assert_raises_rpc_error(-6, msg, self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=zero_value) + msg = "Invalid amount" + # 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, msg, self.nodes[2].sendtoaddress, address=address, amount=1.0, fee_rate=invalid_value) + # Test fee_rate values that cannot be represented in duff/B. + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + assert_raises_rpc_error(-3, msg, self.nodes[2].sendtoaddress, address=address, amount=10, fee_rate=invalid_value) + # Test fee_rate out of range (negative number). + assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[2].sendtoaddress, address=address, amount=1.0, fee_rate=-1) + # Test type error. + for invalid_value in [True, {"foo": "bar"}]: + assert_raises_rpc_error(-3, NOT_A_NUMBER_OR_STRING, self.nodes[2].sendtoaddress, address=address, amount=1.0, fee_rate=invalid_value) + + self.log.info("Test sendtoaddress raises if an invalid conf_target or estimate_mode is passed") + for target, mode in product([-1, 0, 1009], ["economical", "conservative"]): + assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", # max value of 1008 per src/policy/fees.h + self.nodes[2].sendtoaddress, address=address, amount=1, conf_target=target, estimate_mode=mode) + for target, mode in product([-1, 0], ["dash/kb", "duff/b"]): + assert_raises_rpc_error(-8, 'Invalid estimate_mode parameter, must be one of: "unset", "economical", "conservative"', + self.nodes[2].sendtoaddress, address=address, amount=1, conf_target=target, estimate_mode=mode) # 2. Import address from node2 to node1 self.nodes[1].importaddress(address_to_import) @@ -547,7 +560,7 @@ class WalletTest(BitcoinTestFramework): ] chainlimit = 6 for m in maintenance: - self.log.info("check " + m) + self.log.info("Test " + m) self.stop_nodes() # set lower ancestor limit for later self.start_node(0, [m, "-limitancestorcount=" + str(chainlimit)]) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 78ffc4bb33..81292298cd 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -5,6 +5,8 @@ """Test the send RPC command.""" from decimal import Decimal, getcontext +from itertools import product + from test_framework.authproxy import JSONRPCException from test_framework.descriptors import descsum_create from test_framework.test_framework import BitcoinTestFramework @@ -12,7 +14,7 @@ from test_framework.util import ( assert_equal, assert_fee_amount, assert_greater_than, - assert_raises_rpc_error + assert_raises_rpc_error, ) class WalletSendTest(BitcoinTestFramework): @@ -29,8 +31,8 @@ class WalletSendTest(BitcoinTestFramework): self.skip_if_no_wallet() def test_send(self, from_wallet, to_wallet=None, amount=None, data=None, - arg_conf_target=None, arg_estimate_mode=None, - conf_target=None, estimate_mode=None, add_to_wallet=None, psbt=None, + arg_conf_target=None, arg_estimate_mode=None, arg_fee_rate=None, + conf_target=None, estimate_mode=None, fee_rate=None, add_to_wallet=None, psbt=None, inputs=None, add_inputs=None, include_unsafe=None, change_address=None, change_position=None, include_watching=None, locktime=None, lock_unspents=None, subtract_fee_from_outputs=None, expect_error=None): @@ -66,6 +68,8 @@ class WalletSendTest(BitcoinTestFramework): options["conf_target"] = conf_target if estimate_mode is not None: options["estimate_mode"] = estimate_mode + if fee_rate is not None: + options["fee_rate"] = fee_rate if inputs is not None: options["inputs"] = inputs if add_inputs is not None: @@ -89,16 +93,19 @@ class WalletSendTest(BitcoinTestFramework): options = None if expect_error is None: - res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, options=options) + res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, fee_rate=arg_fee_rate, options=options) else: try: assert_raises_rpc_error(expect_error[0], expect_error[1], from_wallet.send, - outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, options=options) + outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, fee_rate=arg_fee_rate, options=options) except AssertionError: # Provide debug info if the test fails self.log.error("Unexpected successful result:") + self.log.error(arg_conf_target) + self.log.error(arg_estimate_mode) + self.log.error(arg_fee_rate) self.log.error(options) - res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, options=options) + res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, fee_rate=arg_fee_rate, options=options) self.log.error(res) if "txid" in res and add_to_wallet: self.log.error("Transaction details:") @@ -271,8 +278,10 @@ class WalletSendTest(BitcoinTestFramework): assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], self.nodes[1].decodepsbt(res2["psbt"])["fee"]) # but not at the same time - self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical", - conf_target=1, estimate_mode="economical", add_to_wallet=False, expect_error=(-8,"Use either conf_target and estimate_mode or the options dictionary to control fee rate")) + for mode in ["unset", "economical", "conservative"]: + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical", + conf_target=1, estimate_mode=mode, add_to_wallet=False, + expect_error=(-8, "Pass conf_target and estimate_mode either as arguments or in the options object, but not both")) self.log.info("Create PSBT from watch-only wallet w3, sign with w2...") res = self.test_send(from_wallet=w3, to_wallet=w1, amount=1) @@ -297,12 +306,82 @@ class WalletSendTest(BitcoinTestFramework): res = w2.walletprocesspsbt(res["psbt"]) assert res["complete"] - self.log.info("Set fee rate...") - res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=2, estimate_mode="duff/b", add_to_wallet=False) + self.log.info("Test setting explicit fee rate") + res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate="1", add_to_wallet=False) + res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate="1", add_to_wallet=False) + assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], self.nodes[1].decodepsbt(res2["psbt"])["fee"]) + + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, add_to_wallet=False) + fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] + assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00007")) + + # "unset" and None are treated the same for estimate_mode + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=2, estimate_mode="unset", add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00002")) - self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=-1, estimate_mode="duff/b", - expect_error=(-3, "Amount out of range")) + + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=4.531, add_to_wallet=False) + fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] + assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00004531")) + + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=3, add_to_wallet=False) + fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] + assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00003")) + + # Test that passing fee_rate as both an argument and an option raises. + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=1, fee_rate=1, add_to_wallet=False, + expect_error=(-8, "Pass the fee_rate either as an argument, or in the options object, but not both")) + + assert_raises_rpc_error(-8, "Use fee_rate (duff/B) instead of feeRate", w0.send, {w1.getnewaddress(): 1}, 6, "conservative", 1, {"feeRate": 0.01}) + + assert_raises_rpc_error(-3, "Unexpected key totalFee", w0.send, {w1.getnewaddress(): 1}, 6, "conservative", 1, {"totalFee": 0.01}) + + for target, mode in product([-1, 0, 1009], ["economical", "conservative"]): + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=target, estimate_mode=mode, + expect_error=(-8, "Invalid conf_target, must be between 1 and 1008")) # max value of 1008 per src/policy/fees.h + msg = 'Invalid estimate_mode parameter, must be one of: "unset", "economical", "conservative"' + for target, mode in product([-1, 0], ["dash/kb", "dufff/b"]): + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=target, estimate_mode=mode, expect_error=(-8, msg)) + for mode in ["", "foo", Decimal("3.141592")]: + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode=mode, expect_error=(-8, msg)) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=0.1, arg_estimate_mode=mode, expect_error=(-8, msg)) + assert_raises_rpc_error(-8, msg, w0.send, {w1.getnewaddress(): 1}, 0.1, mode) + + for mode in ["economical", "conservative"]: + for k, v in {"string": "true", "bool": True, "object": {"foo": "bar"}}.items(): + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=v, estimate_mode=mode, + expect_error=(-3, f"Expected type number for conf_target, got {k}")) + + # Test setting explicit fee rate just below the minimum of 1 duff/B. + self.log.info("Explicit fee rate raises RPC error 'fee rate too low' if fee_rate of 0.99999999 is passed") + msg = "Fee rate (0.999 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)" + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=0.999, expect_error=(-4, msg)) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=0.999, expect_error=(-4, msg)) + + self.log.info("Explicit fee rate raises if invalid fee_rate is passed") + # Test fee_rate with zero values. + msg = "Fee rate (0.000 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)" + for zero_value in [0, 0.000, 0.00000000, "0", "0.000", "0.00000000"]: + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=zero_value, expect_error=(-4, msg)) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=zero_value, expect_error=(-4, msg)) + msg = "Invalid amount" + # 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"]: + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=invalid_value, expect_error=(-3, msg)) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=invalid_value, expect_error=(-3, msg)) + # Test fee_rate values that cannot be represented in duff/B. + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=invalid_value, expect_error=(-3, msg)) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=invalid_value, expect_error=(-3, msg)) + # Test fee_rate out of range (negative number). + msg = "Amount out of range" + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=-1, expect_error=(-3, msg)) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=-1, expect_error=(-3, msg)) + # Test type error. + msg = "Amount is not a number or string" + for invalid_value in [True, {"foo": "bar"}]: + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=invalid_value, expect_error=(-3, msg)) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=invalid_value, expect_error=(-3, msg)) # TODO: Return hex if fee rate is below -maxmempool # res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode="duff/b", add_to_wallet=False) diff --git a/test/functional/wallet_watchonly.py b/test/functional/wallet_watchonly.py index 2b91edf22f..c3ac93229f 100755 --- a/test/functional/wallet_watchonly.py +++ b/test/functional/wallet_watchonly.py @@ -2,7 +2,7 @@ # Copyright (c) 2018-2019 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Test createwallet arguments. +"""Test createwallet watchonly arguments. """ from test_framework.blocktools import COINBASE_MATURITY @@ -51,6 +51,11 @@ class CreateWalletWatchonlyTest(BitcoinTestFramework): assert_equal(len(wo_wallet.listtransactions()), 1) assert_equal(wo_wallet.getbalance(include_watchonly=False), 0) + self.log.info('Test sending from a watch-only wallet raises RPC error') + msg = "Error: Private keys are disabled for this wallet" + assert_raises_rpc_error(-4, msg, wo_wallet.sendtoaddress, a1, 0.1) + assert_raises_rpc_error(-4, msg, wo_wallet.sendmany, amounts={a1: 0.1}) + self.log.info('Testing listreceivedbyaddress watch-only defaults') result = wo_wallet.listreceivedbyaddress() assert_equal(len(result), 1)