diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index d1790c351c..5f14b67ba0 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"); 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/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/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 79343ff868..b308579e41 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -221,7 +221,8 @@ static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const Un if (!estimate_mode.isNull() && estimate_mode.get_str() != "unset") { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate"); } - cc.m_feerate = CFeeRate(AmountFromValue(fee_rate), COIN); + // 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; } diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 809769fbfc..a1c1334e94 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -5,6 +5,8 @@ """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 ( @@ -717,17 +719,16 @@ class RawTransactionsTest(BitcoinTestFramework): 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)}) - # Test that funding non-standard "zero-fee" transactions is valid. - result5 = self.nodes[3].fundrawtransaction(rawtx, {"fee_rate": 0}) - result6 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 0}) 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) - assert_fee_amount(result5['fee'], count_bytes(result5['hex']), 0) - assert_fee_amount(result6['fee'], count_bytes(result6['hex']), 0) + + # 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) @@ -761,11 +762,16 @@ class RawTransactionsTest(BitcoinTestFramework): 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, {param: "", "add_inputs": True}) + node.fundrawtransaction, rawtx, {"fee_rate": invalid_value, "add_inputs": True}) self.log.info("Test min fee rate checks are bypassed with fundrawtxn, e.g. a fee_rate under 1 sat/vB is allowed") - node.fundrawtransaction(rawtx, {"fee_rate": 0.99999999, "add_inputs": True}) + node.fundrawtransaction(rawtx, {"fee_rate": 0.999, "add_inputs": True}) node.fundrawtransaction(rawtx, {"feeRate": 0.00000999, "add_inputs": True}) self.log.info("- raises RPC error if both feeRate and fee_rate are passed") diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index db3500089b..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, @@ -115,14 +117,14 @@ class PSBTTest(BitcoinTestFramework): assert_approx(res2["fee"], 0.04, 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}) + 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 in ["fee_rate", "feeRate"]: - assert_equal(self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {param: 0, "add_inputs": True})["fee"], 0) + 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)}: @@ -132,8 +134,14 @@ class PSBTTest(BitcoinTestFramework): 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, {param: "", "add_inputs": True}) + 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)", diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 013a77078e..6ba8a44e24 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -17,6 +17,7 @@ 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" @@ -277,12 +278,25 @@ class WalletTest(BitcoinTestFramework): # 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.99999999) + self.nodes[2].sendmany, amounts={address: 10}, fee_rate=0.999) - self.log.info("Test sendmany raises if fee_rate of 0 or -1 is passed") - assert_raises_rpc_error(-6, "Fee rate (0.000 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)", - self.nodes[2].sendmany, amounts={address: 10}, fee_rate=0) + 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"]): @@ -425,7 +439,7 @@ class WalletTest(BitcoinTestFramework): 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 sat/vB)") + 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() @@ -462,10 +476,23 @@ class WalletTest(BitcoinTestFramework): 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 fee_rate of 0 or -1 is passed") - assert_raises_rpc_error(-6, "Fee rate (0.000 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)", - self.nodes[2].sendtoaddress, address=address, amount=10, fee_rate=0) + 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"]): diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 89f9b554a2..81292298cd 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -347,22 +347,41 @@ class WalletSendTest(BitcoinTestFramework): 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", "dash/kb", "duff/b"]: - self.log.debug("{}".format(mode)) - for k, v in {"string": "true", "object": {"foo": "bar"}}.items(): + 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, "Expected type number for conf_target, got {}".format(k))) + expect_error=(-3, f"Expected type number for conf_target, got {k}")) - # Test setting explicit fee rate just below the minimum and at zero. + # 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") - self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=0.99999999, - expect_error=(-4, "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, arg_fee_rate=0.99999999, - expect_error=(-4, "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, - expect_error=(-4, "Fee rate (0.000 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, arg_fee_rate=0, - expect_error=(-4, "Fee rate (0.000 duff/B) is lower than the minimum fee rate setting (1.000 duff/B)")) + 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)