foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

wallet: override minfee checks (fOverrideFeeRate) for fee_rate

Browse source 
in RPCs fundrawtransaction and walletcreatefundedpsbt only.

This provides the existing feeRate (BTC/kvB) behavior in these two RPCs to the
new fee_rate (sat/vB) param also.

See these two GitHub review discussions for more info:
https://github.com/bitcoin/bitcoin/pull/10706/#discussion_r126560525
https://github.com/bitcoin/bitcoin/pull/20305#discussion_r520032533
This commit is contained in:
Jon Atack 2020-11-12 11:16:48 +01:00
parent 9a670b4f07
commit 05e82d86b0
No known key found for this signature in database
GPG key ID: 4F5721B3D0E3921D
3 changed files with 37 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
src/wallet/rpcwallet.cpp
View file

@ -198,16 +198,18 @@ static std::string LabelFromValue(const UniValue& value)
* Update coin control with fee estimation based on the given parameters * Update coin control with fee estimation based on the given parameters
* *
* @param[in] pwallet Wallet pointer * @param[in] pwallet Wallet pointer
* @param[in,out] cc Coin control which is to be updated * @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] 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 * 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"; * @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 * 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; * @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 values * 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 * @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& conf_target, const UniValue& estimate_mode, const UniValue& fee_rate) 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 (!fee_rate.isNull()) { if (!fee_rate.isNull()) {
if (!conf_target.isNull() && conf_target.get_int() > 0) { if (!conf_target.isNull() && conf_target.get_int() > 0) {
@ -216,7 +218,14 @@ static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const U
if (!estimate_mode.isNull() && !estimate_mode.get_str().empty()) { if (!estimate_mode.isNull() && !estimate_mode.get_str().empty()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate"); throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate");
} }
cc.m_feerate = CFeeRate(AmountFromValue(fee_rate), COIN); 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 = fee_rate_in_sat_vb;
// Default RBF to true for explicit fee_rate, if unset. // Default RBF to true for explicit fee_rate, if unset.
if (cc.m_signal_bip125_rbf == nullopt) cc.m_signal_bip125_rbf = true; if (cc.m_signal_bip125_rbf == nullopt) cc.m_signal_bip125_rbf = true;
return; return;
@ -504,7 +513,7 @@ static RPCHelpMan sendtoaddress()
// We also enable partial spend avoidance if reuse avoidance is set. // We also enable partial spend avoidance if reuse avoidance is set.
coin_control.m_avoid_partial_spends |= coin_control.m_avoid_address_reuse; coin_control.m_avoid_partial_spends |= coin_control.m_avoid_address_reuse;
SetFeeEstimateMode(pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[9]); 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); EnsureWalletIsUnlocked(pwallet);
@ -932,7 +941,7 @@ static RPCHelpMan sendmany()
coin_control.m_signal_bip125_rbf = request.params[5].get_bool(); coin_control.m_signal_bip125_rbf = request.params[5].get_bool();
} }
SetFeeEstimateMode(pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[8]); 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; std::vector<CRecipient> recipients;
ParseRecipients(sendTo, subtractFeeFromAmount, recipients); ParseRecipients(sendTo, subtractFeeFromAmount, recipients);
@ -3049,7 +3058,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 // 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 // the user could have gotten from another RPC command prior to now
@ -3154,7 +3163,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
if (options.exists("replaceable")) { if (options.exists("replaceable")) {
coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool();
} }
SetFeeEstimateMode(pwallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"]); SetFeeEstimateMode(pwallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee);
} }
} else { } else {
// if options is null and not a bool // if options is null and not a bool
@ -3275,7 +3284,7 @@ static RPCHelpMan fundrawtransaction()
CCoinControl coin_control; CCoinControl coin_control;
// Automatically select (additional) coins. Can be overridden by options.add_inputs. // Automatically select (additional) coins. Can be overridden by options.add_inputs.
coin_control.m_add_inputs = true; 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); UniValue result(UniValue::VOBJ);
result.pushKV("hex", EncodeHexTx(CTransaction(tx))); result.pushKV("hex", EncodeHexTx(CTransaction(tx)));
@ -3482,7 +3491,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
if (options.exists("replaceable")) { if (options.exists("replaceable")) {
coin_control.m_signal_bip125_rbf = options["replaceable"].get_bool(); coin_control.m_signal_bip125_rbf = options["replaceable"].get_bool();
} }
SetFeeEstimateMode(pwallet, coin_control, conf_target, options["estimate_mode"], options["fee_rate"]); 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 // Make sure the results are valid at least up to the most recent block
@ -4146,7 +4155,7 @@ static RPCHelpMan send()
// Automatically select coins, unless at least one is manually selected. Can // Automatically select coins, unless at least one is manually selected. Can
// be overridden by options.add_inputs. // be overridden by options.add_inputs.
coin_control.m_add_inputs = rawTx.vin.size() == 0; 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; bool add_to_wallet = true;
if (options.exists("add_to_wallet")) { if (options.exists("add_to_wallet")) {
@ -4433,7 +4442,7 @@ static RPCHelpMan walletcreatefundedpsbt()
// Automatically select coins, unless at least one is manually selected. Can // Automatically select coins, unless at least one is manually selected. Can
// be overridden by options.add_inputs. // be overridden by options.add_inputs.
coin_control.m_add_inputs = rawTx.vin.size() == 0; 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 // Make a blank psbt
PartiallySignedTransaction psbtx(rawTx); PartiallySignedTransaction psbtx(rawTx);

11
test/functional/rpc_fundrawtransaction.py
View file

@ -752,7 +752,7 @@ class RawTransactionsTest(BitcoinTestFramework):
node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": n, "add_inputs": True}) node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": n, "add_inputs": True})
self.log.info("Test invalid fee rate settings") self.log.info("Test invalid fee rate settings")
assert_raises_rpc_error(-4, "Fee rate (0.000 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)", assert_raises_rpc_error(-8, "Invalid fee_rate 0.000 sat/vB (must be greater than 0)",
node.fundrawtransaction, rawtx, {"fee_rate": 0, "add_inputs": True}) node.fundrawtransaction, rawtx, {"fee_rate": 0, "add_inputs": True})
assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kvB (must be greater than 0)", assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kvB (must be greater than 0)",
node.fundrawtransaction, rawtx, {"feeRate": 0, "add_inputs": True}) node.fundrawtransaction, rawtx, {"feeRate": 0, "add_inputs": True})
@ -766,12 +766,9 @@ class RawTransactionsTest(BitcoinTestFramework):
assert_raises_rpc_error(-3, "Invalid amount", assert_raises_rpc_error(-3, "Invalid amount",
node.fundrawtransaction, rawtx, {"fee_rate": "", "add_inputs": True}) node.fundrawtransaction, rawtx, {"fee_rate": "", "add_inputs": True})
# Test setting explicit fee rate just below the minimum. self.log.info("Test min fee rate checks are bypassed with fundrawtxn, e.g. a fee_rate under 1 sat/vB is allowed")
self.log.info("- raises RPC error 'fee rate too low' if fee_rate of 0.99999999 sat/vB is passed") node.fundrawtransaction(rawtx, {"fee_rate": 0.99999999, "add_inputs": True})
msg = "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)" node.fundrawtransaction(rawtx, {"feeRate": 0.00000999, "add_inputs": True})
assert_raises_rpc_error(-4, msg, node.fundrawtransaction, rawtx, {"fee_rate": 0.99999999, "add_inputs": True})
# This feeRate test only passes if `coinControl.fOverrideFeeRate = true` in wallet/rpcwallet.cpp::FundTransaction is removed.
# assert_raises_rpc_error(-4, msg, node.fundrawtransaction, rawtx, {"feeRate": 0.00000999, "add_inputs": True})
self.log.info("- raises RPC error if both feeRate and fee_rate are passed") self.log.info("- raises RPC error if both feeRate and fee_rate are passed")
assert_raises_rpc_error(-8, "Cannot specify both fee_rate (sat/vB) and feeRate (BTC/kvB)", assert_raises_rpc_error(-8, "Cannot specify both fee_rate (sat/vB) and feeRate (BTC/kvB)",

12
test/functional/rpc_psbt.py
View file

@ -192,9 +192,14 @@ class PSBTTest(BitcoinTestFramework):
assert_approx(res1["fee"], 0.055, 0.005) assert_approx(res1["fee"], 0.055, 0.005)
res2 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, "add_inputs": True}) res2 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, "add_inputs": True})
assert_approx(res2["fee"], 0.055, 0.005) assert_approx(res2["fee"], 0.055, 0.005)
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.99999999, "add_inputs": True})
assert_approx(res3["fee"], 0.00000381, 0.0000001)
res4 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.00000999, "add_inputs": True})
assert_approx(res4["fee"], 0.00000381, 0.0000001)
self.log.info("Test invalid fee rate settings") self.log.info("Test invalid fee rate settings")
assert_raises_rpc_error(-4, "Fee rate (0.000 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)", assert_raises_rpc_error(-8, "Invalid fee_rate 0.000 sat/vB (must be greater than 0)",
self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": 0, "add_inputs": True}) self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": 0, "add_inputs": True})
assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kvB (must be greater than 0)", assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kvB (must be greater than 0)",
self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"feeRate": 0, "add_inputs": True}) self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"feeRate": 0, "add_inputs": True})
@ -244,11 +249,6 @@ class PSBTTest(BitcoinTestFramework):
assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", # max value of 1008 per src/policy/fees.h 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.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"estimate_mode": mode, "conf_target": n, "add_inputs": True})
# Test setting explicit fee rate just below the minimum.
self.log.info("- raises RPC error 'fee rate too low' if feerate_sat_vb of 0.99999999 is passed")
assert_raises_rpc_error(-4, "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)",
self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": 0.99999999, "add_inputs": True})
self.log.info("Test walletcreatefundedpsbt with too-high fee rate produces total fee well above -maxtxfee and raises RPC error") 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 # previously this was silently capped at -maxtxfee
for bool_add, outputs_array in {True: outputs, False: [{self.nodes[1].getnewaddress(): 1}]}.items(): for bool_add, outputs_array in {True: outputs, False: [{self.nodes[1].getnewaddress(): 1}]}.items():