diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 3b02da632f..d1790c351c 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -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/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index fdea130b65..4f39fd5fd5 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -221,7 +221,7 @@ static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const Un 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"); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Selected estimate_mode %s requires a fee rate to be specified in conf_target", estimate_mode.get_str())); } CAmount fee_rate = AmountFromValue(estimate_param); @@ -426,7 +426,8 @@ 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)\n" + "or fee rate (for " + CURRENCY_UNIT + "/kB and " + CURRENCY_ATOM + "/B estimate modes)"}, {"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" @@ -921,7 +922,8 @@ 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)\n" + "or fee rate (for " + CURRENCY_UNIT + "/kB and " + CURRENCY_ATOM + "/B estimate modes)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra infomration about the transaction."}, @@ -3378,7 +3380,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) { // 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 @@ -3533,7 +3535,8 @@ 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)\n" + "or fee rate (for " + CURRENCY_UNIT + "/kB and " + CURRENCY_ATOM + "/B estimate modes)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, }, @@ -4177,7 +4180,8 @@ 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)\n" + "or fee rate (for " + CURRENCY_UNIT + "/kB and " + CURRENCY_ATOM + "/B estimate modes)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", @@ -4189,7 +4193,8 @@ 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)\n" + "or fee rate (for " + CURRENCY_UNIT + "/kB and " + CURRENCY_ATOM + "/B estimate modes)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, {"include_watching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only.\n" @@ -4205,7 +4210,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.", @@ -4279,7 +4284,7 @@ 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); @@ -4534,7 +4539,8 @@ 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)\n" + "or fee rate (for " + CURRENCY_UNIT + "/kB and " + CURRENCY_ATOM + "/B estimate modes)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, }, diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index bf33d1092d..6ac389e53f 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -8,6 +8,7 @@ from decimal import Decimal 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, @@ -93,6 +94,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.test_op_return() self.test_watchonly() self.test_all_watched_funds() + self.test_feerate_with_conf_target_and_estimate_mode() self.test_option_feerate() self.test_address_reuse() self.test_option_subtract_fee_from_outputs() @@ -718,6 +720,59 @@ class RawTransactionsTest(BitcoinTestFramework): 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) + def test_feerate_with_conf_target_and_estimate_mode(self): + self.log.info("Test fundrawtxn passing an explicit fee rate using conf_target and estimate_mode") + node = self.nodes[3] + # Make sure there is exactly one input so coin selection can't skew the result. + assert_equal(len(node.listunspent(1)), 1) + inputs = [] + outputs = {node.getnewaddress() : 1} + rawtx = node.createrawtransaction(inputs, outputs) + + for unit, fee_rate in {"dash/kb": 0.1, "duff/b": 10000}.items(): + self.log.info("Test fundrawtxn with conf_target {} estimate_mode {} produces expected fee".format(fee_rate, unit)) + # With no arguments passed, expect fee of 225 sats/b. + assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000225, vspan=0.00000001) + # Expect fee to be 10,000x higher when explicit fee 10,000x greater is specified. + result = node.fundrawtransaction(rawtx, {"conf_target": fee_rate, "estimate_mode": unit}) + assert_approx(result["fee"], vexp=0.0225, vspan=0.0001) + + for field, fee_rate in {"conf_target": 0.1, "estimate_mode": "duff/b"}.items(): + self.log.info("Test fundrawtxn raises RPC error if both feeRate and {} are passed".format(field)) + assert_raises_rpc_error( + -8, "Cannot specify both {} and feeRate".format(field), + lambda: node.fundrawtransaction(rawtx, {"feeRate": 0.1, field: fee_rate})) + + 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), + lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": v, "conf_target": 0.1})) + for mode in ["foo", Decimal("3.141592")]: + assert_raises_rpc_error(-8, "Invalid estimate_mode parameter", + lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": 0.1})) + + self.log.info("Test fundrawtxn with invalid conf_target settings") + for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]: + 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), + lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": v})) + if mode in ["dash/kb", "duff/b"]: + assert_raises_rpc_error(-3, "Amount out of range", + lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": -1})) + assert_raises_rpc_error(-4, "Fee rate (0.00000000 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)", + lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": 0})) + else: + for n in [-1, 0, 1009]: + assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", + lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": n})) + + for unit, fee_rate in {"duff/B": 0.99999999, "DASH/kB": 0.00000999}.items(): + self.log.info("- raises RPC error 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit)) + assert_raises_rpc_error(-4, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)", + lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": unit, "conf_target": fee_rate, "add_inputs": True})) + + def test_address_reuse(self): """Test no address reuse occurs.""" self.log.info("Test fundrawtxn does not reuse addresses") diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index cb5bd668a8..e6753c1564 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -94,8 +94,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 +108,62 @@ 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) + self.log.info("Test walletcreatefundedpsbt feeRate of 0.1 DASH/kB produces a total fee at or slightly below -maxtxfee (~0.06650000)") + res = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, "add_inputs": True}) 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 - # feeRate of 10 DASH / KB produces a total fee well above -maxtxfee + self.log.info("Test walletcreatefundedpsbt explicit fee rate with conf_target and estimate_mode") + for unit, fee_rate in {"dash/kb": 0.1, "duff/b": 10000}.items(): + fee = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"conf_target": fee_rate, "estimate_mode": unit, "add_inputs": True})["fee"] + self.log.info("- conf_target {}, estimate_mode {} produces fee {} at or slightly below -maxtxfee (~0.05290000)".format(fee_rate, unit, fee)) + assert_approx(fee, vexp=0.04, vspan=0.03) + + for field, fee_rate in {"conf_target": 0.1, "estimate_mode": "duff/b"}.items(): + self.log.info("- raises RPC error if both feeRate and {} are passed".format(field)) + assert_raises_rpc_error(-8, "Cannot specify both {} and feeRate".format(field), + lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, field: fee_rate, "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), + lambda: 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", + lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": 0.1, "add_inputs": True})) + + self.log.info("- raises RPC error if estimate_mode is passed without a conf_target") + for unit in ["DUFF/B", "DASH/KB"]: + assert_raises_rpc_error(-8, "Selected estimate_mode {} requires a fee rate to be specified in conf_target".format(unit), + lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": unit})) + + self.log.info("- raises RPC error with invalid conf_target settings") + for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]: + 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), + lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": v, "add_inputs": True})) + if mode in ["dash/kb", "duff/b"]: + assert_raises_rpc_error(-3, "Amount out of range", + lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": -1, "add_inputs": True})) + assert_raises_rpc_error(-4, "Fee rate (0.00000000 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)", + lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": 0, "add_inputs": True})) + else: + for n in [-1, 0, 1009]: + assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", + lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": mode, "conf_target": n, "add_inputs": True})) + + for unit, fee_rate in {"DUFF/B": 0.99999999, "DASH/KB": 0.00000999}.items(): + self.log.info("- raises RPC error 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit)) + assert_raises_rpc_error(-4, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)", + lambda: self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"estimate_mode": unit, "conf_target": fee_rate, "add_inputs": True})) + + self.log.info("Test walletcreatefundedpsbt feeRate of 10 DASH/kB 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(): + assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", + self.nodes[1].walletcreatefundedpsbt, inputs, outputs_array, 0, {"feeRate": 10, "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..7621dd05a6 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -221,6 +221,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) @@ -239,9 +241,9 @@ class WalletTest(BitcoinTestFramework): self.start_node(3, self.nodes[3].extra_args) self.connect_nodes(0, 3) - # Sendmany with explicit fee (DASH/kB) + self.log.info("Test case-insensitive explicit fee rate (sendmany as DASH/kB)") # Throw if no conf_target provided - assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate", + assert_raises_rpc_error(-8, "Selected estimate_mode dash/kB requires a fee rate to be specified in conf_target", self.nodes[2].sendmany, amounts={ address: 10 }, estimate_mode='dash/kB') @@ -265,9 +267,9 @@ class WalletTest(BitcoinTestFramework): node_0_bal += Decimal('10') assert_equal(self.nodes[0].getbalance(), node_0_bal) - # Sendmany with explicit fee (DUFF/B) + self.log.info("Test case-insensitive explicit fee rate (sendmany as duff/B)") # Throw if no conf_target provided - assert_raises_rpc_error(-8, "Selected estimate_mode requires a fee rate", + assert_raises_rpc_error(-8, "Selected estimate_mode duff/b requires a fee rate to be specified in conf_target", self.nodes[2].sendmany, amounts={ address: 10 }, estimate_mode='duff/b') @@ -293,6 +295,11 @@ class WalletTest(BitcoinTestFramework): node_0_bal += Decimal('10') assert_equal(self.nodes[0].getbalance(), node_0_bal) + # Test setting explicit fee rate just below the minimum. + for unit, fee_rate in {"DASH/kB": 0.00000999, "duff/B": 0.99999999}.items(): + self.log.info("Test sendmany raises 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit)) + assert_raises_rpc_error(-6, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)", + self.nodes[2].sendmany, amounts={address: 10}, estimate_mode=unit, conf_target=fee_rate) # check if we can list zero value tx as available coins # 1. create raw_tx @@ -423,15 +430,14 @@ 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.log.info("Test case-insensitive explicit fee rate (sendtoaddress as DASH/kB)") self.nodes[0].generate(1) self.sync_all(self.nodes[0:3]) 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", + assert_raises_rpc_error(-8, "Selected estimate_mode dash/Kb requires a fee rate to be specified in conf_target", self.nodes[2].sendtoaddress, address=address, amount=1.0, @@ -457,15 +463,15 @@ class WalletTest(BitcoinTestFramework): fee = prebalance - postbalance - Decimal('1') assert_fee_amount(fee, tx_size, Decimal('0.00002500')) - # send with explicit duff/b fee self.sync_all(self.nodes[0:3]) - self.log.info("test explicit fee (sendtoaddress as duff/b)") + + self.log.info("Test case-insensitive explicit fee rate (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", + assert_raises_rpc_error(-8, "Selected estimate_mode duff/b requires a fee rate to be specified in conf_target", self.nodes[2].sendtoaddress, address=address, amount=1.0, @@ -491,6 +497,12 @@ class WalletTest(BitcoinTestFramework): fee = prebalance - postbalance - Decimal('1') assert_fee_amount(fee, tx_size, Decimal('0.00002000')) + # Test setting explicit fee rate just below the minimum. + for unit, fee_rate in {"DASH/kB": 0.00000999, "sat/B": 0.99999999}.items(): + self.log.info("Test sendtoaddress raises 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit)) + assert_raises_rpc_error(-6, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)", + self.nodes[2].sendtoaddress, address=address, amount=1, estimate_mode=unit, conf_target=fee_rate) + # 2. Import address from node2 to node1 self.nodes[1].importaddress(address_to_import) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 78ffc4bb33..734ed9d13d 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -97,6 +97,8 @@ class WalletSendTest(BitcoinTestFramework): 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(options) res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, options=options) self.log.error(res) @@ -271,8 +273,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", "dash/kb", "duff/b"]: + 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, "Use either conf_target and estimate_mode or the options dictionary to control fee rate")) 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 +301,61 @@ class WalletSendTest(BitcoinTestFramework): res = w2.walletprocesspsbt(res["psbt"]) assert res["complete"] - self.log.info("Set fee rate...") + self.log.info("Test setting explicit fee rate") + res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical", add_to_wallet=False) + res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=1, estimate_mode="economical", 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, conf_target=0.00007, estimate_mode="dash/kb", add_to_wallet=False) + fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] + assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00007")) + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=2, estimate_mode="duff/b", 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_conf_target=0.00004531, arg_estimate_mode="dash/kb", 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_conf_target=3, arg_estimate_mode="duff/b", add_to_wallet=False) + fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] + assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00003")) + + # TODO: This test should pass with all modes, e.g. with the next line uncommented, for consistency with the other explicit feerate RPCs. + # for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]: + for mode in ["dash/kb", "duff/b"]: + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=-1, estimate_mode=mode, + expect_error=(-3, "Amount out of range")) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0, estimate_mode=mode, + expect_error=(-4, "Fee rate (0.00000000 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)")) + + 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, "Invalid estimate_mode parameter")) + # TODO: these 2 equivalent sends with an invalid estimate_mode arg should both fail, but they do not...why? + # self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=0.1, arg_estimate_mode=mode, + # expect_error=(-8, "Invalid estimate_mode parameter")) + # assert_raises_rpc_error(-8, "Invalid estimate_mode parameter", lambda: w0.send({w1.getnewaddress(): 1}, 0.1, mode)) + + # TODO: These tests should pass for consistency with the other explicit feerate RPCs, but they do not. + # for mode in ["unset", "economical", "conservative", "dash/kb", "duff/b"]: + # self.log.debug("{}".format(mode)) + # for k, v in {"string": "", "object": {"foo": "bar"}}.items(): + # self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=v, estimate_mode=mode, + # expect_error=(-3, "Expected type number for conf_target, got {}".format(k))) + + # TODO: error should use duff/B instead of DASH/kB if duff/B is selected. + # Test setting explicit fee rate just below the minimum. + for unit, fee_rate in {"duff/B": 0.99999999, "DASH/kB": 0.00000999}.items(): + self.log.info("Explicit fee rate raises RPC error 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit)) + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=fee_rate, estimate_mode=unit, + expect_error=(-4, "Fee rate (0.00000999 DASH/kB) is lower than the minimum fee rate setting (0.00001000 DASH/kB)")) + + self.log.info("Explicit fee rate raises RPC error if estimate_mode is passed without a conf_target") + for unit, fee_rate in {"duff/B": 100, "DASH/kB": 0.001}.items(): + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, estimate_mode=unit, + expect_error=(-8, "Selected estimate_mode {} requires a fee rate to be specified in conf_target".format(unit))) # 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)