From 01e41aa1fb40a1185f9e36c4c5c06036e54c9b83 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 21 Nov 2020 07:47:06 +0100 Subject: [PATCH] Merge #20426: wallet: allow zero-fee fundrawtransaction/walletcreatefundedpsbt and other fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 9f08780dd7946b63476e9736745131db8e7f4e93 Use the correct incremental fee constant in bumpfee help (Jon Atack) 3f1e10b2b1cd11f7112fbad6355464bd4adbbc5c Update feeRate (BTC/kvB) to fee_rate (sat/vB) in wallet_bumpfee (Jon Atack) 1b3d7009280595108eb22ac1188bc4367804fc5d Allow zero-fee fundrawtxn and walletcreatefundedpsbt calls (Jon Atack) Pull request description: - Fixes https://github.com/bitcoin/bitcoin/pull/20305/files#r525406176. A check to raise an error on zero-fee txns was mistakenly extended in a0d4957 from the bumpfee and send{toaddress, many} RPCs to also include fundrawtransaction and walletcreatefundedpsbt. This commit re-overrides zero fee rate checking for these two RPCs, not only for the feeRate (BTC/kvB) arg to return to previous behavior, but also for the new fee_rate (sat/vB) arg. Negative fee rates will still raise "amount out of range" by the MoneyRange check in src/bitcoin-tx.cpp::AmountFromValue. - Updates a wallet bumpfee test from feeRate (BTC/kvB) to fee_rate (sat/vB) - Fixes https://github.com/bitcoin/bitcoin/pull/20305/files#r525405363 to use the correct incremental fee rate constant in the bumpfee help (thanks Marco Falke for the catch) and rectifies "1.000 sat/vB sat/vB" in the help to "1.000 sat/vB" ACKs for top commit: MarcoFalke: review ACK 9f08780dd7946b63476e9736745131db8e7f4e93 🐾 promag: Code review ACK 9f08780dd7946b63476e9736745131db8e7f4e93. Xekyo: Code review reACK 9f08780dd7946b63476e9736745131db8e7f4e93. Tree-SHA512: 413dfb4f23ebaf3d2ef210dd04610a843272e64eabba428699f5de4d646a86ac4911dab66b5e2f5ebea53b76e4be8347ef40824c1592c750d5eaa12579d3cdf6 --- src/wallet/rpcwallet.cpp | 17 +++-------------- test/functional/rpc_fundrawtransaction.py | 16 +++++++++------- test/functional/rpc_psbt.py | 15 ++++++++------- test/functional/wallet_send.py | 8 +++++++- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 3f00992cf8..fe7250201b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -224,14 +223,8 @@ static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const Un if (!estimate_mode.isNull() && !estimate_mode.get_str().empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate"); } - 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::DUFF_B))); - } - cc.fOverrideFeeRate = true; - } - cc.m_feerate = fee_rate_in_sat_vb; + cc.m_feerate = CFeeRate(AmountFromValue(fee_rate), COIN); + if (override_min_fee) cc.fOverrideFeeRate = true; return; } if (!estimate_mode.isNull() && !FeeModeFromString(estimate_mode.get_str(), cc.m_fee_mode)) { @@ -3474,11 +3467,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, if (options.exists("estimate_mode")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and 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::DASH_KB))); - } - coinControl.m_feerate = fee_rate; + coinControl.m_feerate = CFeeRate(AmountFromValue(options["feeRate"])); coinControl.fOverrideFeeRate = true; } diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index eb78dfb395..d183ae8cae 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -717,11 +717,17 @@ 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": 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(result2['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(result3['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) # With no arguments passed, expect fee of 225 satoshis. assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000225, vspan=0.00000001) @@ -748,19 +754,15 @@ class RawTransactionsTest(BitcoinTestFramework): node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": n, "add_inputs": True}) self.log.info("Test invalid fee rate settings") - assert_raises_rpc_error(-8, "Invalid fee_rate 0.000 duff/B (must be greater than 0)", - node.fundrawtransaction, rawtx, {"fee_rate": 0, "add_inputs": True}) - assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 DASH/kB (must be greater than 0)", - node.fundrawtransaction, rawtx, {"feeRate": 0, "add_inputs": True}) for param, value in {("fee_rate", 100000), ("feeRate", 1.000)}: assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", node.fundrawtransaction, rawtx, {param: value, "add_inputs": True}) assert_raises_rpc_error(-3, "Amount out of range", - node.fundrawtransaction, rawtx, {"fee_rate": -1, "add_inputs": True}) + node.fundrawtransaction, rawtx, {param: -1, "add_inputs": True}) assert_raises_rpc_error(-3, "Amount is not a number or string", - node.fundrawtransaction, rawtx, {"fee_rate": {"foo": "bar"}, "add_inputs": True}) + node.fundrawtransaction, rawtx, {param: {"foo": "bar"}, "add_inputs": True}) assert_raises_rpc_error(-3, "Invalid amount", - node.fundrawtransaction, rawtx, {"fee_rate": "", "add_inputs": True}) + node.fundrawtransaction, rawtx, {param: "", "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}) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 2bdf1d8b7f..8e73e3b8d8 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -113,26 +113,27 @@ class PSBTTest(BitcoinTestFramework): assert_approx(res1["fee"], 0.04, 0.005) res2 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, "add_inputs": True}) assert_approx(res2["fee"], 0.04, 0.005) + 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.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) + self.log.info("Test invalid fee rate settings") - assert_raises_rpc_error(-8, "Invalid fee_rate 0.000 duff/B (must be greater than 0)", - self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": 0, "add_inputs": True}) - assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 DASH/kB (must be greater than 0)", - self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"feeRate": 0, "add_inputs": True}) for param, value in {("fee_rate", 100000), ("feeRate", 1)}: assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: value, "add_inputs": True}) assert_raises_rpc_error(-3, "Amount out of range", - self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": -1, "add_inputs": True}) + 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, {"fee_rate": {"foo": "bar"}, "add_inputs": True}) + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: {"foo": "bar"}, "add_inputs": True}) assert_raises_rpc_error(-3, "Invalid amount", - self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": "", "add_inputs": True}) + self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: "", "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_send.py b/test/functional/wallet_send.py index 1eef86be90..b73dbb1540 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -354,10 +354,16 @@ class WalletSendTest(BitcoinTestFramework): 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))) - # Test setting explicit fee rate just below the minimum. + # Test setting explicit fee rate just below the minimum and at zero. 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)")) # 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)