From fa69c2c78455fd0dc436018fece9ff7fc83a180d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 17 Nov 2020 20:08:30 +0100 Subject: wallet: Do not treat default constructed types as None-type --- src/wallet/rpcwallet.cpp | 18 ++++++++---------- test/functional/wallet_basic.py | 6 ++---- test/functional/wallet_send.py | 9 ++++----- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d3cf487c4f..3ff90e8609 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -201,11 +201,9 @@ static std::string LabelFromValue(const UniValue& value) * @param[in] wallet Wallet reference * @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 + * if present, both conf_target and estimate_mode must either be null, or "unset" * @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 @@ -213,10 +211,10 @@ static std::string LabelFromValue(const UniValue& value) static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const UniValue& conf_target, const UniValue& estimate_mode, const UniValue& fee_rate, bool override_min_fee) { if (!fee_rate.isNull()) { - if (!conf_target.isNull() && conf_target.get_int() > 0) { + if (!conf_target.isNull()) { 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 (!estimate_mode.isNull() && !estimate_mode.get_str().empty()) { + if (!estimate_mode.isNull() && estimate_mode.get_str() != "unset") { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate"); } CFeeRate fee_rate_in_sat_vb{CFeeRate(AmountFromValue(fee_rate), COIN)}; @@ -473,8 +471,8 @@ static RPCHelpMan sendtoaddress() + 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.1 BTC with a fee rate of 1.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 null \"unset\" null 1.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" @@ -4081,10 +4079,10 @@ static RPCHelpMan send() RPCExamples{"" "\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.1 " + CURRENCY_ATOM + "/vB using positional arguments\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' null \"unset\" 1.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") + + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' null \"unset\" null '{\"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" diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index ead56046a4..ac4a6e4948 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -235,8 +235,7 @@ class WalletTest(BitcoinTestFramework): fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 explicit_fee_rate_btc_kvb = Decimal(fee_rate_btc_kvb) / 1000 - # Passing conf_target 0, estimate_mode "" as placeholder arguments should allow fee_rate to apply. - txid = self.nodes[2].sendmany(amounts={address: 10}, conf_target=0, estimate_mode="", fee_rate=fee_rate_sat_vb) + txid = self.nodes[2].sendmany(amounts={address: 10}, fee_rate=fee_rate_sat_vb) self.nodes[2].generate(1) self.sync_all(self.nodes[0:3]) balance = self.nodes[2].getbalance() @@ -407,8 +406,7 @@ class WalletTest(BitcoinTestFramework): fee_rate_sat_vb = 2 fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 - # Passing conf_target 0, estimate_mode "" as placeholder arguments should allow fee_rate to apply. - txid = self.nodes[2].sendtoaddress(address=address, amount=amount, conf_target=0, estimate_mode="", fee_rate=fee_rate_sat_vb) + txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=fee_rate_sat_vb) tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex']) self.nodes[0].generate(1) self.sync_all(self.nodes[0:3]) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 5da71c85f9..376cfdec6c 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -260,17 +260,16 @@ class WalletSendTest(BitcoinTestFramework): res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=1, add_to_wallet=False) assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], self.nodes[1].decodepsbt(res2["psbt"])["fee"]) - # Passing conf_target 0, estimate_mode "" as placeholder arguments should allow fee_rate to apply. - res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0, estimate_mode="", fee_rate=7, add_to_wallet=False) + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, 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, fee_rate=2, add_to_wallet=False) + # "unset" and None are treated the same for estimate_mode + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=2, estimate_mode="unset", add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00002")) - # Passing conf_target 0, estimate_mode "" as placeholder arguments should allow fee_rate to apply. - res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=0, arg_estimate_mode="", arg_fee_rate=4.531, add_to_wallet=False) + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=4.531, add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00004531")) -- cgit v1.2.3