diff options
author | MarcoFalke <falke.marco@gmail.com> | 2020-11-17 13:49:04 +0100 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2020-11-17 13:49:12 +0100 |
commit | 80e32e120ee4ba7e5b458338682cf1130964218f (patch) | |
tree | 091db154c695582747a1fecd1c33dc72e53d6a0f /src/wallet | |
parent | e7986c51bc7afeca8f79f286fb5d950f469a8866 (diff) | |
parent | 05e82d86b09d914ebce05dbc92a7299cb026847b (diff) |
Merge #20305: wallet: introduce fee_rate sat/vB param/option
05e82d86b09d914ebce05dbc92a7299cb026847b wallet: override minfee checks (fOverrideFeeRate) for fee_rate (Jon Atack)
9a670b4f07a6140de809d73cbd7f3e614eb6ea74 wallet: update sendtoaddress, send RPC examples with fee_rate (Jon Atack)
be481b72e24fb6834bd674cd8daee67c6938b42d wallet: use MIN_RELAY_TX_FEE in bumpfee help (Jon Atack)
449b730579566459e350703611629e63e54657ed wallet: provide valid values if invalid estimate mode passed (Jon Atack)
6da3afbaee5809ebf6d88efaa3958c505c2d71c7 wallet: update remaining rpcwallet fee rate units to BTC/kvB (Jon Atack)
173b5b5fe07d45be5a1e5bc7a5df996f20ab1e85 wallet: update fee rate units, use sat/vB for fee_rate error messages (Jon Atack)
7f9835a05abf3e168ad93e7195cbaa4bf61b9b07 wallet: remove fee rates from conf_target helps (Jon Atack)
b7994c01e9a3251536fe6538a22f614774eec82d wallet: add fee_rate unit warnings to bumpfee (Jon Atack)
410e471fa42d3db04e8879c71f8c824dcc151a83 wallet: remove redundant bumpfee fee_rate checks (Jon Atack)
a0d495747320c79b27a83c216dcc526ac8df8f24 wallet: introduce fee_rate (sat/vB) param/option (Jon Atack)
e21212f01b7c41eba13b0479b252053cf482bc1f wallet: remove unneeded WALLET_BTC_KB_TO_SAT_B constant (Jon Atack)
6112cf20d43b0be34fe0edce2ac3e6b27cae1bbe wallet: add CFeeRate ctor doxygen documentation (Jon Atack)
3f7279161347543ce4e997d78ea89a4043491145 wallet: fix bug in RPC send options (Jon Atack)
Pull request description:
This PR builds on #11413 and #20220 to address #19543.
- replace overloading the conf_target and estimate_mode params with `fee_rate` in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs
- allow non-actionable conf_target value of `0` and estimate_mode value of `""` to be passed to use `fee_rate` as a positional argument, in addition to as a named argument
- fix a bug in the experimental send RPC described in https://github.com/bitcoin/bitcoin/pull/20220#discussion_r513789526 where args were not being passed correctly into the options values
- update the feerate error message units for these RPCs from BTC/kB to sat/vB
- update the test coverage, help docs, doxygen docs, and some of the RPC examples
- other changes to address the excellent review feedback
See this wallet meeting log for more context: http://www.erisian.com.au/bitcoin-core-dev/log-2020-11-06.html#l-309
ACKs for top commit:
achow101:
re-ACK 05e82d8
MarcoFalke:
review ACK 05e82d86b0 did not test and found a few style nits, which can be fixed later 🍯
Xekyo:
tACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Sjors:
utACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Tree-SHA512: a4ee5f184ada53f1840b2923d25873bda88c5a2ae48e67eeea2417a0b35154798cfdb3c147b05dd56bd6608a784e1b91623bb985ee2ab9ef2baaec22206d0a9c
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/rpcwallet.cpp | 223 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 2 |
2 files changed, 126 insertions, 99 deletions
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8b08d7a5e6..6efa3d0c27 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -29,6 +29,7 @@ #include <util/translation.h> #include <util/url.h> #include <util/vector.h> +#include <validation.h> #include <wallet/coincontrol.h> #include <wallet/context.h> #include <wallet/feebumper.h> @@ -48,8 +49,6 @@ using interfaces::FoundBlock; static const std::string WALLET_ENDPOINT_BASE = "/wallet/"; static const std::string HELP_REQUIRING_PASSPHRASE{"\nRequires wallet passphrase to be set with walletpassphrase call if wallet is encrypted.\n"}; -static const uint32_t WALLET_BTC_KB_TO_SAT_B = COIN / 1000; // 1 sat / B = 0.00001 BTC / kB - static inline bool GetAvoidReuseFlag(const CWallet* const pwallet, const UniValue& param) { bool can_avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); bool avoid_reuse = param.isNull() ? can_avoid_reuse : param.get_bool(); @@ -199,36 +198,44 @@ static std::string LabelFromValue(const UniValue& value) /** * Update coin control with fee estimation based on the given parameters * - * @param[in] pwallet Wallet pointer - * @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] pwallet Wallet pointer + * @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; + * if a fee_rate is present, 0 is allowed here as a no-op positional placeholder + * @param[in] estimate_mode UniValue string; fee estimation mode, valid values are "unset", "economical" or "conservative"; + * if a fee_rate is present, "" is allowed here as a no-op positional placeholder + * @param[in] fee_rate UniValue real; fee rate in sat/vB; + * if a fee_rate is present, both conf_target and estimate_mode must either be null, or no-op + * @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* pwallet, CCoinControl& cc, const UniValue& estimate_mode, const UniValue& estimate_param) +static void SetFeeEstimateMode(const CWallet* pwallet, 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() && conf_target.get_int() > 0) { + 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 (cc.m_fee_mode == FeeEstimateMode::BTC_KB || cc.m_fee_mode == FeeEstimateMode::SAT_B) { - if (estimate_param.isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Selected estimate_mode %s requires a fee rate to be specified in conf_target", estimate_mode.get_str())); + if (!estimate_mode.isNull() && !estimate_mode.get_str().empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate"); } - - CAmount fee_rate = AmountFromValue(estimate_param); - if (cc.m_fee_mode == FeeEstimateMode::SAT_B) { - fee_rate /= WALLET_BTC_KB_TO_SAT_B; + CFeeRate fee_rate_in_sat_vb{CFeeRate(AmountFromValue(fee_rate), COIN)}; + if (override_min_fee) { + if (fee_rate_in_sat_vb <= CFeeRate(0)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid fee_rate %s (must be greater than 0)", fee_rate_in_sat_vb.ToString(FeeEstimateMode::SAT_VB))); + } + cc.fOverrideFeeRate = true; } - - cc.m_feerate = CFeeRate(fee_rate); - - // default RBF to true for explicit fee rate modes + cc.m_feerate = fee_rate_in_sat_vb; + // Default RBF to true for explicit fee_rate, if unset. if (cc.m_signal_bip125_rbf == nullopt) cc.m_signal_bip125_rbf = true; - } else if (!estimate_param.isNull()) { - cc.m_confirm_target = ParseConfirmTarget(estimate_param, pwallet->chain().estimateMaxBlocks()); + return; + } + 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, pwallet->chain().estimateMaxBlocks()); } } @@ -441,12 +448,12 @@ static RPCHelpMan sendtoaddress() {"subtractfeefromamount", RPCArg::Type::BOOL, /* default */ "false", "The fee will be deducted from the amount being sent.\n" "The recipient will receive less bitcoins than you enter in the amount field."}, {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n" - "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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 + "/vB."}, {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra information about the transaction."}, }, { @@ -462,12 +469,17 @@ static RPCHelpMan sendtoaddress() }, }, 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\"") + "\nSend 0.1 BTC\n" + + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1") + + "\nSend 0.1 BTC 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 true 6 economical") + + "\nSend 0.1 BTC with a fee rate of 1 " + CURRENCY_ATOM + "/vB, subtract fee from amount, BIP125-replaceable, using positional arguments\n" + + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"drinks\" \"room77\" true true 0 \"\" 1") + + "\nSend 0.2 BTC 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 BTC with a fee rate of 25 " + CURRENCY_ATOM + "/vB 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 replaceable=true avoid_reuse=true comment=\"2 pizzas\" comment_to=\"jeremy\" verbose=true") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { @@ -502,7 +514,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[7], request.params[6]); + SetFeeEstimateMode(pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[9], /* override_min_fee */ false); EnsureWalletIsUnlocked(pwallet); @@ -516,7 +528,7 @@ static RPCHelpMan sendtoaddress() std::vector<CRecipient> recipients; ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); - bool verbose = request.params[9].isNull() ? false: request.params[9].get_bool(); + const bool verbose{request.params[10].isNull() ? false : request.params[10].get_bool()}; return SendMoney(pwallet, coin_control, recipients, mapValue, verbose); }, @@ -870,10 +882,10 @@ static RPCHelpMan sendmany() }, }, {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n" - "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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 + "/vB."}, {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra infomration about the transaction."}, }, { @@ -930,11 +942,11 @@ static RPCHelpMan sendmany() coin_control.m_signal_bip125_rbf = request.params[5].get_bool(); } - SetFeeEstimateMode(pwallet, coin_control, request.params[7], request.params[6]); + SetFeeEstimateMode(pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[8], /* override_min_fee */ false); std::vector<CRecipient> recipients; ParseRecipients(sendTo, subtractFeeFromAmount, recipients); - bool verbose = request.params[8].isNull() ? false : request.params[8].get_bool(); + const bool verbose{request.params[9].isNull() ? false : request.params[9].get_bool()}; return SendMoney(pwallet, coin_control, recipients, std::move(mapValue), verbose); }, @@ -2311,7 +2323,7 @@ static RPCHelpMan settxfee() "\nSet the transaction fee per kB for this wallet. Overrides the global -paytxfee command line parameter.\n" "Can be deactivated by passing 0 as the fee. In that case automatic fee selection will be used by default.\n", { - {"amount", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The transaction fee in " + CURRENCY_UNIT + "/kB"}, + {"amount", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The transaction fee in " + CURRENCY_UNIT + "/kvB"}, }, RPCResult{ RPCResult::Type::BOOL, "", "Returns true if successful" @@ -2434,7 +2446,7 @@ static RPCHelpMan getwalletinfo() {RPCResult::Type::NUM, "keypoolsize", "how many new keys are pre-generated (only counts external keys)"}, {RPCResult::Type::NUM, "keypoolsize_hd_internal", "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"}, {RPCResult::Type::NUM_TIME, "unlocked_until", /* optional */ true, "the " + UNIX_EPOCH_TIME + " until which the wallet is unlocked for transfers, or 0 if the wallet is locked (only present for passphrase-encrypted wallets)"}, - {RPCResult::Type::STR_AMOUNT, "paytxfee", "the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB"}, + {RPCResult::Type::STR_AMOUNT, "paytxfee", "the transaction fee configuration, set in " + CURRENCY_UNIT + "/kvB"}, {RPCResult::Type::STR_HEX, "hdseedid", /* optional */ true, "the Hash160 of the HD seed (only present when HD is enabled)"}, {RPCResult::Type::BOOL, "private_keys_enabled", "false if privatekeys are disabled for this wallet (enforced watch-only wallet)"}, {RPCResult::Type::BOOL, "avoid_reuse", "whether this wallet tracks clean/dirty coins in terms of reuse"}, @@ -3047,7 +3059,7 @@ static RPCHelpMan listunspent() }; } -void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, const UniValue& options, CCoinControl& coinControl) +void FundTransaction(CWallet* const pwallet, 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 @@ -3080,7 +3092,8 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f {"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)}, @@ -3127,15 +3140,21 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f lockUnspents = (options.exists("lock_unspents") ? options["lock_unspents"] : options["lockUnspents"]).get_bool(); } - if (options.exists("feeRate")) - { + if (options.exists("feeRate")) { + if (options.exists("fee_rate")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both fee_rate (" + CURRENCY_ATOM + "/vB) and feeRate (" + CURRENCY_UNIT + "/kvB)"); + } 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"); } - coinControl.m_feerate = CFeeRate(AmountFromValue(options["feeRate"])); + CFeeRate fee_rate(AmountFromValue(options["feeRate"])); + if (fee_rate <= CFeeRate(0)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid feeRate %s (must be greater than 0)", fee_rate.ToString(FeeEstimateMode::BTC_KVB))); + } + coinControl.m_feerate = fee_rate; coinControl.fOverrideFeeRate = true; } @@ -3145,7 +3164,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f if (options.exists("replaceable")) { coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); } - SetFeeEstimateMode(pwallet, coinControl, options["estimate_mode"], options["conf_target"]); + SetFeeEstimateMode(pwallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee); } } else { // if options is null and not a bool @@ -3202,7 +3221,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 + "/vB."}, + {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_UNIT + "/kvB."}, {"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 bitcoins than you enter in their corresponding amount field.\n" @@ -3213,8 +3233,7 @@ static RPCHelpMan fundrawtransaction() }, {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n" "Allows this transaction to be replaced by a transaction with higher fees"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n" - "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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\"") + "\""}, }, @@ -3266,7 +3285,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))); @@ -3374,35 +3393,38 @@ RPCHelpMan signrawtransactionwithwallet() static RPCHelpMan bumpfee_helper(std::string method_name) { bool want_psbt = method_name == "psbtbumpfee"; + const std::string incremental_fee{CFeeRate(DEFAULT_MIN_RELAY_TX_FEE).ToString(FeeEstimateMode::SAT_VB)}; return RPCHelpMan{method_name, "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" + std::string(want_psbt ? "Returns a PSBT instead of creating and signing a new transaction.\n" : "") + "An opt-in RBF transaction with the given txid must be in the wallet.\n" - "The command will pay the additional fee by reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist.\n" + "The command will pay the additional fee by reducing change outputs or adding inputs when necessary.\n" + "It may add a new change output if one does not already exist.\n" "All inputs in the original transaction will be included in the replacement transaction.\n" "The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n" "By default, the new fee will be calculated automatically using the estimatesmartfee RPC.\n" "The user can specify a confirmation target for estimatesmartfee.\n" - "Alternatively, the user can specify a fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction.\n" + "Alternatively, the user can specify a fee rate in " + CURRENCY_ATOM + "/vB for the new transaction.\n" "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" - "returned by getnetworkinfo) to enter the node's mempool.\n", + "returned by getnetworkinfo) to enter the node's mempool.\n" + "* WARNING: before version 0.21, fee_rate was in " + CURRENCY_UNIT + "/kvB. As of 0.21, fee_rate is in " + CURRENCY_ATOM + "/vB. *\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n" - "or fee rate (for " + CURRENCY_UNIT + "/kB and " + CURRENCY_ATOM + "/B estimate modes)"}, - {"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'conf_target'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + "/kB.\n" - "Specify a fee rate instead of relying on the built-in fee estimator.\n" - "Must be at least 0.0001 " + CURRENCY_UNIT + "/kB higher than the current transaction fee rate.\n"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target in blocks\n"}, + {"fee_rate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", + "\nSpecify a fee rate in " + CURRENCY_ATOM + "/vB instead of relying on the built-in fee estimator.\n" + "Must be at least " + incremental_fee + " " + CURRENCY_ATOM + "/vB higher than the current transaction fee rate.\n" + "WARNING: before version 0.21, fee_rate was in " + CURRENCY_UNIT + "/kvB. As of 0.21, fee_rate is in " + CURRENCY_ATOM + "/vB.\n"}, {"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n" "marked bip-125 replaceable. If true, the sequence numbers in the transaction will\n" "be left unchanged from the original. If false, any input sequence numbers in the\n" "original transaction that were less than 0xfffffffe will be increased to 0xfffffffe\n" "so the new transaction will not be explicitly bip-125 replaceable (though it may\n" "still be replaceable in practice, for example if it has unconfirmed ancestors which\n" - "are replaceable)."}, + "are replaceable).\n"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, }, @@ -3455,7 +3477,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) { {"confTarget", UniValueType(UniValue::VNUM)}, {"conf_target", UniValueType(UniValue::VNUM)}, - {"fee_rate", UniValueType(UniValue::VNUM)}, + {"fee_rate", UniValueType()}, // will be checked by AmountFromValue() in SetFeeEstimateMode() {"replaceable", UniValueType(UniValue::VBOOL)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, }, @@ -3467,22 +3489,10 @@ static RPCHelpMan bumpfee_helper(std::string method_name) auto conf_target = options.exists("confTarget") ? options["confTarget"] : options["conf_target"]; - if (!conf_target.isNull()) { - if (options.exists("fee_rate")) { - 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."); - } - } else if (options.exists("fee_rate")) { - CFeeRate fee_rate(AmountFromValue(options["fee_rate"])); - if (fee_rate <= CFeeRate(0)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid fee_rate %s (must be greater than 0)", fee_rate.ToString())); - } - coin_control.m_feerate = fee_rate; - } - if (options.exists("replaceable")) { coin_control.m_signal_bip125_rbf = options["replaceable"].get_bool(); } - SetFeeEstimateMode(pwallet, coin_control, options["estimate_mode"], conf_target); + SetFeeEstimateMode(pwallet, coin_control, conf_target, options["estimate_mode"], options["fee_rate"], /* override_min_fee */ false); } // Make sure the results are valid at least up to the most recent block @@ -4018,10 +4028,10 @@ static RPCHelpMan send() }, }, }, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n" - "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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 + "/vB."}, {"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."}, @@ -4029,10 +4039,10 @@ static RPCHelpMan send() {"change_address", RPCArg::Type::STR_HEX, /* default */ "pool address", "The bitcoin address to receive the change"}, {"change_position", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, {"change_type", RPCArg::Type::STR, /* default */ "set by -changetype", "The output type to use. Only valid if change_address is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."}, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n" - "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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 + "/vB."}, {"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."}, @@ -4069,18 +4079,25 @@ static RPCHelpMan send() } }, RPCExamples{"" - "\nSend with a fee rate of 1 satoshi per byte\n" - + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.1}' 1 sat/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 BTC 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 BTC with a fee rate of 1 " + CURRENCY_ATOM + "/vB using positional arguments\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' 0 \"\" 1\n") + + "Send 0.2 BTC with a fee rate of 1 " + CURRENCY_ATOM + "/vB using the options argument\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' '{\"fee_rate\": 1}'\n") + + "Send 0.3 BTC with a fee rate of 25 " + CURRENCY_ATOM + "/vB 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 + UniValueType(), // outputs (ARR or OBJ, checked later) + UniValue::VNUM, // conf_target + UniValue::VSTR, // estimate_mode + UniValue::VNUM, // fee_rate + UniValue::VOBJ, // options }, true ); @@ -4088,18 +4105,28 @@ static RPCHelpMan send() if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); - 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("conf_target") || options.exists("estimate_mode")) { 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 + "/vB) instead of feeRate"); + } if (options.exists("changeAddress")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_address"); } @@ -4129,7 +4156,7 @@ static RPCHelpMan send() // 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, 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")) { @@ -4357,7 +4384,8 @@ static RPCHelpMan walletcreatefundedpsbt() {"change_type", RPCArg::Type::STR, /* default */ "set by -changetype", "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."}, {"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"}, - {"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 + "/vB."}, + {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set, fall back to wallet fee estimation", "Specify a fee rate in " + CURRENCY_UNIT + "/kvB."}, {"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" "Those recipients will receive less bitcoins than you enter in their corresponding amount field.\n" @@ -4368,8 +4396,7 @@ static RPCHelpMan walletcreatefundedpsbt() }, {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n" "Allows this transaction to be replaced by a transaction with higher fees"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet -txconfirmtarget", "Confirmation target (in blocks)\n" - "or fee rate (for " + CURRENCY_UNIT + "/kB and " + 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\"") + "\""}, }, @@ -4416,7 +4443,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); @@ -4549,9 +4576,9 @@ static const CRPCCommand commands[] = { "wallet", "lockunspent", &lockunspent, {"unlock","transactions"} }, { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, - { "wallet", "send", &send, {"outputs","conf_target","estimate_mode","options"} }, - { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","comment","subtractfeefrom","replaceable","conf_target","estimate_mode","verbose"} }, - { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode","avoid_reuse","verbose"} }, + { "wallet", "send", &send, {"outputs","conf_target","estimate_mode","fee_rate","options"} }, + { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","comment","subtractfeefrom","replaceable","conf_target","estimate_mode","fee_rate","verbose"} }, + { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode","avoid_reuse","fee_rate","verbose"} }, { "wallet", "sethdseed", &sethdseed, {"newkeypool","seed"} }, { "wallet", "setlabel", &setlabel, {"address","label"} }, { "wallet", "settxfee", &settxfee, {"amount"} }, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3ce8272fb9..ff8bfff872 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2818,7 +2818,7 @@ bool CWallet::CreateTransactionInternal( // 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 && nFeeRateNeeded > *coin_control.m_feerate) { - error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(), nFeeRateNeeded.ToString()); + error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), nFeeRateNeeded.ToString(FeeEstimateMode::SAT_VB)); return false; } |