diff --git a/doc/release-notes-16383.md b/doc/release-notes-16383.md new file mode 100644 index 0000000000..8015715167 --- /dev/null +++ b/doc/release-notes-16383.md @@ -0,0 +1,8 @@ +RPC changes +----------- + +RPCs which have an `include_watchonly` argument or `includeWatching` +option now default to `true` for watch-only wallets. Affected RPCs +are: `getbalance`, `listreceivedbyaddress`, `listreceivedbylabel`, +`listtransactions`, `listsinceblock`, `gettransaction`, +`walletcreatefundedpsbt`, and `fundrawtransaction`. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e5424e82ea..ac903017a1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -62,6 +62,23 @@ static inline bool GetAvoidReuseFlag(CWallet * const pwallet, const UniValue& pa return avoid_reuse; } + +/** Used by RPC commands that have an include_watchonly parameter. + * We default to true for watchonly wallets if include_watchonly isn't + * explicitly set. + */ +static bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& pwallet) +{ + if (include_watchonly.isNull()) { + // if include_watchonly isn't explicitly set, then check if we have a watchonly wallet + return pwallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); + } + + // otherwise return whatever include_watchonly was set to + return include_watchonly.get_bool(); +} + + /** Checks if a CKey is in the given CWallet compressed or otherwise*/ /* bool HaveKey(const SigningProvider& wallet, const CKey& key) @@ -737,7 +754,7 @@ static UniValue getbalance(const JSONRPCRequest& request) {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."}, {"minconf", RPCArg::Type::NUM, /* default */ "0", "Only include transactions confirmed at least this many times."}, {"addlocked", RPCArg::Type::BOOL, /* default */ "false", "Whether to include transactions locked via InstantSend in the wallet's balance."}, - {"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Also include balance in watch-only addresses (see 'importaddress')"}, + {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also include balance in watch-only addresses (see 'importaddress')"}, {"avoid_reuse", RPCArg::Type::BOOL, /* default */ pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) ? "true" : "unavailable", "Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."}, }, RPCResult{ @@ -775,10 +792,7 @@ static UniValue getbalance(const JSONRPCRequest& request) fAddLocked = addlocked.get_bool(); } - bool include_watchonly = false; - if (!request.params[3].isNull() && request.params[3].get_bool()) { - include_watchonly = true; - } + bool include_watchonly = ParseIncludeWatchonly(request.params[3], *pwallet); bool avoid_reuse = GetAvoidReuseFlag(pwallet, request.params[4]); const auto bal = pwallet->GetBalance(min_depth, avoid_reuse, fAddLocked); @@ -1048,9 +1062,9 @@ static UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bo fIncludeEmpty = params[2].get_bool(); isminefilter filter = ISMINE_SPENDABLE; - if(!params[3].isNull()) - if(params[3].get_bool()) - filter = filter | ISMINE_WATCH_ONLY; + if (ParseIncludeWatchonly(params[3], *pwallet)) { + filter |= ISMINE_WATCH_ONLY; + } bool has_filtered_address = false; CTxDestination filtered_address = CNoDestination(); @@ -1191,7 +1205,7 @@ static UniValue listreceivedbyaddress(const JSONRPCRequest& request) {"minconf", RPCArg::Type::NUM, /* default */ "1", "The minimum number of confirmations before payments are included."}, {"addlocked", RPCArg::Type::BOOL, /* default */ "false", "Whether to include transactions locked via InstantSend."}, {"include_empty", RPCArg::Type::BOOL, /* default */ "false", "Whether to include addresses that haven't received any payments."}, - {"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Whether to include watch-only addresses (see 'importaddress')."}, + {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses (see 'importaddress')"}, {"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If present, only return information on this address."}, }, RPCResult{ @@ -1242,7 +1256,7 @@ static UniValue listreceivedbylabel(const JSONRPCRequest& request) {"minconf", RPCArg::Type::NUM, /* default */ "1", "The minimum number of confirmations before payments are included."}, {"addlocked", RPCArg::Type::BOOL, /* default */ "false", "Whether to include transactions locked via InstantSend."}, {"include_empty", RPCArg::Type::BOOL, /* default */ "false", "Whether to include labels that haven't received any payments."}, - {"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Whether to include watch-only addresses (see 'importaddress')."}, + {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses (see 'importaddress')"}, }, RPCResult{ RPCResult::Type::ARR, "", "", @@ -1402,7 +1416,7 @@ static UniValue listtransactions(const JSONRPCRequest& request) "with the specified label, or \"*\" to disable filtering and return all transactions."}, {"count", RPCArg::Type::NUM, /* default */ "10", "The number of transactions to return"}, {"skip", RPCArg::Type::NUM, /* default */ "0", "The number of transactions to skip"}, - {"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Include transactions to watch-only addresses (see 'importaddress')"}, + {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Include transactions to watch-only addresses (see 'importaddress')"}, }, RPCResult{ RPCResult::Type::ARR, "", "", @@ -1465,9 +1479,10 @@ static UniValue listtransactions(const JSONRPCRequest& request) if (!request.params[2].isNull()) nFrom = request.params[2].get_int(); isminefilter filter = ISMINE_SPENDABLE; - if(!request.params[3].isNull()) - if(request.params[3].get_bool()) - filter = filter | ISMINE_WATCH_ONLY; + + if (ParseIncludeWatchonly(request.params[3], *pwallet)) { + filter |= ISMINE_WATCH_ONLY; + } if (nCount < 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative count"); @@ -1512,7 +1527,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) { {"blockhash", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If set, the block hash to list transactions since, otherwise list all transactions."}, {"target_confirmations", RPCArg::Type::NUM, /* default */ "1", "Return the nth block hash from the main chain. e.g. 1 would mean the best block hash. Note: this is not used as a filter, but only affects [lastblock] in the return value"}, - {"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Include transactions to watch-only addresses (see 'importaddress')"}, + {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Include transactions to watch-only addresses (see 'importaddress')"}, {"include_removed", RPCArg::Type::BOOL, /* default */ "true", "Show transactions that were removed due to a reorg in the \"removed\" array\n" " (not guaranteed to work on pruned nodes)"}, }, @@ -1591,8 +1606,8 @@ static UniValue listsinceblock(const JSONRPCRequest& request) } } - if (!request.params[2].isNull() && request.params[2].get_bool()) { - filter = filter | ISMINE_WATCH_ONLY; + if (ParseIncludeWatchonly(request.params[2], *pwallet)) { + filter |= ISMINE_WATCH_ONLY; } bool include_removed = (request.params[3].isNull() || request.params[3].get_bool()); @@ -1647,7 +1662,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) "\nGet detailed information about in-wallet transaction \n", { {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, - {"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Whether to include watch-only addresses in balance calculation and details[]"}, + {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses in balance calculation and details[]"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", Cat(Cat>( @@ -1704,9 +1719,10 @@ static UniValue gettransaction(const JSONRPCRequest& request) hash.SetHex(request.params[0].get_str()); isminefilter filter = ISMINE_SPENDABLE; - if(!request.params[1].isNull()) - if(request.params[1].get_bool()) - filter = filter | ISMINE_WATCH_ONLY; + + if (ParseIncludeWatchonly(request.params[1], *pwallet)) { + filter |= ISMINE_WATCH_ONLY; + } UniValue entry(UniValue::VOBJ); auto it = pwallet->mapWallet.find(hash); @@ -3138,8 +3154,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f if (options.exists("changePosition")) change_position = options["changePosition"].get_int(); - if (options.exists("includeWatching")) - coinControl.fAllowWatchOnly = options["includeWatching"].get_bool(); + coinControl.fAllowWatchOnly = ParseIncludeWatchonly(options["includeWatching"], *pwallet); if (options.exists("lockUnspents")) lockUnspents = options["lockUnspents"].get_bool(); @@ -3167,6 +3182,9 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f } } } + } else { + // if options is null and not a bool + coinControl.fAllowWatchOnly = ParseIncludeWatchonly(NullUniValue, *pwallet); } if (tx.vout.size() == 0) @@ -3213,7 +3231,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) { {"changeAddress", RPCArg::Type::STR, /* default */ "pool address", "The dash address to receive the change"}, {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, - {"includeWatching", RPCArg::Type::BOOL, /* default */ "false", "Also select inputs which are watch only.\n" + {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only.\n" "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, {"lockUnspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, @@ -3845,7 +3863,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) }, }, }, - }, + }, {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs), where none of the keys are duplicated.\n" "That is, each address can only appear once and there can only be one 'data' object.\n" "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" @@ -3868,7 +3886,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) { {"changeAddress", RPCArg::Type::STR_HEX, /* default */ "pool address", "The dash address to receive the change"}, {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, - {"includeWatching", RPCArg::Type::BOOL, /* default */ "false", "Also select inputs which are watch only"}, + {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only"}, {"lockUnspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set: makes wallet determine the fee", "Set a specific fee rate in " + CURRENCY_UNIT + "/kB"}, {"subtractFeeFromOutputs", RPCArg::Type::ARR, /* default */ "empty array", "A json array of integers.\n" diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index e8c17a1009..df695cdff0 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -148,6 +148,8 @@ BASE_SCRIPTS = [ 'wallet_createwallet.py', 'wallet_createwallet.py --usecli', 'wallet_reorgsrestore.py', + 'wallet_watchonly.py', + 'wallet_watchonly.py --usecli', 'interface_http.py', 'interface_rpc.py', 'rpc_psbt.py', diff --git a/test/functional/wallet_watchonly.py b/test/functional/wallet_watchonly.py new file mode 100644 index 0000000000..be8d7714fb --- /dev/null +++ b/test/functional/wallet_watchonly.py @@ -0,0 +1,107 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018-2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test createwallet arguments. +""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error +) + + +class CreateWalletWatchonlyTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + self.supports_cli = True + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + node = self.nodes[0] + + self.nodes[0].createwallet(wallet_name='default') + def_wallet = node.get_wallet_rpc('default') + + a1 = def_wallet.getnewaddress() + wo_change = def_wallet.getnewaddress() + wo_addr = def_wallet.getnewaddress() + + self.nodes[0].createwallet(wallet_name='wo', disable_private_keys=True) + wo_wallet = node.get_wallet_rpc('wo') + + wo_wallet.importpubkey(pubkey=def_wallet.getaddressinfo(wo_addr)['pubkey']) + wo_wallet.importpubkey(pubkey=def_wallet.getaddressinfo(wo_change)['pubkey']) + + # generate some btc for testing + node.generatetoaddress(101, a1) + + # send 1 btc to our watch-only address + txid = def_wallet.sendtoaddress(wo_addr, 1) + self.nodes[0].generate(1) + + # getbalance + self.log.info('include_watchonly should default to true for watch-only wallets') + self.log.info('Testing getbalance watch-only defaults') + assert_equal(wo_wallet.getbalance(), 1) + assert_equal(len(wo_wallet.listtransactions()), 1) + assert_equal(wo_wallet.getbalance(include_watchonly=False), 0) + + self.log.info('Testing listreceivedbyaddress watch-only defaults') + result = wo_wallet.listreceivedbyaddress() + assert_equal(len(result), 1) + assert_equal(result[0]["involvesWatchonly"], True) + result = wo_wallet.listreceivedbyaddress(include_watchonly=False) + assert_equal(len(result), 0) + + self.log.info('Testing listreceivedbylabel watch-only defaults') + result = wo_wallet.listreceivedbylabel() + assert_equal(len(result), 1) + assert_equal(result[0]["involvesWatchonly"], True) + result = wo_wallet.listreceivedbylabel(include_watchonly=False) + assert_equal(len(result), 0) + + self.log.info('Testing listtransactions watch-only defaults') + result = wo_wallet.listtransactions() + assert_equal(len(result), 1) + assert_equal(result[0]["involvesWatchonly"], True) + result = wo_wallet.listtransactions(include_watchonly=False) + assert_equal(len(result), 0) + + self.log.info('Testing listsinceblock watch-only defaults') + result = wo_wallet.listsinceblock() + assert_equal(len(result["transactions"]), 1) + assert_equal(result["transactions"][0]["involvesWatchonly"], True) + result = wo_wallet.listsinceblock(include_watchonly=False) + assert_equal(len(result["transactions"]), 0) + + self.log.info('Testing gettransaction watch-only defaults') + result = wo_wallet.gettransaction(txid) + assert_equal(result["details"][0]["involvesWatchonly"], True) + result = wo_wallet.gettransaction(txid=txid, include_watchonly=False) + assert_equal(len(result["details"]), 0) + + self.log.info('Testing walletcreatefundedpsbt watch-only defaults') + inputs = [] + outputs = [{a1: 0.5}] + options = {'changeAddress': wo_change} + no_wo_options = {'changeAddress': wo_change, 'includeWatching': False} + + result = wo_wallet.walletcreatefundedpsbt(inputs=inputs, outputs=outputs, options=options) + assert_equal("psbt" in result, True) + assert_raises_rpc_error(-4, "Insufficient funds", wo_wallet.walletcreatefundedpsbt, inputs, outputs, 0, no_wo_options) + + self.log.info('Testing fundrawtransaction watch-only defaults') + rawtx = wo_wallet.createrawtransaction(inputs=inputs, outputs=outputs) + result = wo_wallet.fundrawtransaction(hexstring=rawtx, options=options) + assert_equal("hex" in result, True) + assert_raises_rpc_error(-4, "Insufficient funds", wo_wallet.fundrawtransaction, rawtx, no_wo_options) + + + +if __name__ == '__main__': + CreateWalletWatchonlyTest().main()