From 3f7279161347543ce4e997d78ea89a4043491145 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 5 Nov 2020 06:34:01 +0100 Subject: [PATCH] wallet: fix bug in RPC send options when empty, options were not being populated by arguments of the same name found while adding test coverage in 603c0050 --- src/wallet/rpcwallet.cpp | 2 +- test/functional/wallet_send.py | 29 +++++++++++++++-------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ebcab1227d..4a3212e6fe 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4087,7 +4087,7 @@ static RPCHelpMan send() if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); - UniValue options = request.params[3]; + UniValue options{request.params[3].isNull() ? UniValue::VOBJ : request.params[3]}; if (options.exists("feeRate") || options.exists("fee_rate") || options.exists("estimate_mode") || options.exists("conf_target")) { if (!request.params[1].isNull() || !request.params[2].isNull()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Use either conf_target and estimate_mode or the options dictionary to control fee rate"); diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 921a726d4b..82c046a17f 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -5,13 +5,15 @@ """Test the send RPC command.""" from decimal import Decimal, getcontext +from itertools import product + from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_fee_amount, assert_greater_than, - assert_raises_rpc_error + assert_raises_rpc_error, ) class WalletSendTest(BitcoinTestFramework): @@ -271,8 +273,9 @@ def run_test(self): 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", "btc/kb", "sat/b"]: + for target, mode in product([-1, 0, 1009], ["economical", "conservative"]): + self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=target, estimate_mode=mode, + expect_error=(-8, "Invalid conf_target, must be between 1 and 1008")) for mode in ["btc/kb", "sat/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")) @@ -282,17 +285,15 @@ def run_test(self): 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", "btc/kb", "sat/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))) + 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)) + + for mode in ["economical", "conservative", "btc/kb", "sat/b"]: + self.log.debug("{}".format(mode)) + for k, v in {"string": "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))) # TODO: error should use sat/B instead of BTC/kB if sat/B is selected. # Test setting explicit fee rate just below the minimum.