From 9991f49c38c084967ca66791d838c99b42f000eb Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 26 May 2025 12:21:38 -0700 Subject: [PATCH 1/7] test: Watchonly wallets should estimate larger size Test that when a watchonly wallet and the wallet with private keys fund the same tx, the watchonly wallet should use a higher fee since it should be estimating the size to be larger as it assumes the signer cannot grind the R value. --- test/functional/wallet_fundrawtransaction.py | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index a3b19847335..9c5ef057d86 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -152,6 +152,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.test_feerate_rounding() self.test_input_confs_control() self.test_duplicate_outputs() + self.test_watchonly_cannot_grind_r() def test_duplicate_outputs(self): self.log.info("Test deserializing and funding a transaction with duplicate outputs") @@ -1516,5 +1517,28 @@ class RawTransactionsTest(BitcoinTestFramework): wallet.unloadwallet() + def test_watchonly_cannot_grind_r(self): + self.log.info("Test that a watchonly wallet will estimate higher fees for a tx than the wallet with private keys") + self.nodes[0].createwallet("grind") + wallet = self.nodes[0].get_wallet_rpc("grind") + default_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + + self.nodes[0].createwallet(wallet_name="grind_watchonly", disable_private_keys=True) + watchonly = self.nodes[0].get_wallet_rpc("grind_watchonly") + assert_equal(watchonly.importdescriptors(wallet.listdescriptors()["descriptors"])[0]["success"], True) + + # Send to legacy address type so that we will have an ecdsa signature with a measurable effect on the feerate + default_wallet.sendtoaddress(wallet.getnewaddress(address_type="legacy"), 10) + self.generate(self.nodes[0], 1) + + assert_equal(wallet.listunspent(), watchonly.listunspent()) + + ret_addr = default_wallet.getnewaddress() + tx = wallet.createrawtransaction([], [{ret_addr: 5}]) + funded = wallet.fundrawtransaction(hexstring=tx, fee_rate=10) + + watchonly_funded = watchonly.fundrawtransaction(hexstring=tx, fee_rate=10) + assert_greater_than(watchonly_funded["fee"], funded["fee"]) + if __name__ == '__main__': RawTransactionsTest(__file__).main() From d20dc9c6aae089ab926fd135febd69a8f0744a18 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 26 May 2025 12:28:02 -0700 Subject: [PATCH 2/7] wallet: Wallets without private keys cannot grind R --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 36da4396106..6342d466ca2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4038,7 +4038,7 @@ util::Result CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch, bool CWallet::CanGrindR() const { - return !IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER); + return !IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); } bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, MigrationResult& res) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) From e81d95d435744e48615973dc22acce1a291bd20d Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 15 May 2025 15:31:16 -0700 Subject: [PATCH 3/7] wallet: Remove watchonly balances Descriptor wallets do not have mixed mine and watchonly, so there is no need to report a watchonly balance. --- src/wallet/receive.cpp | 8 +------- src/wallet/receive.h | 3 --- src/wallet/rpc/coins.cpp | 6 ++---- test/functional/wallet_balance.py | 6 +----- 4 files changed, 4 insertions(+), 19 deletions(-) diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 1ba85b6b2d8..4c9ffc499d9 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -263,13 +263,10 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) const int tx_depth{wallet.GetTxDepthInMainChain(wtx)}; if (!wallet.IsSpent(outpoint) && (allow_used_addresses || !wallet.IsSpentKey(txo.GetTxOut().scriptPubKey))) { - // Get the amounts for mine and watchonly + // Get the amounts for mine CAmount credit_mine = 0; - CAmount credit_watchonly = 0; if (txo.GetIsMine() == ISMINE_SPENDABLE) { credit_mine = txo.GetTxOut().nValue; - } else if (txo.GetIsMine() == ISMINE_WATCH_ONLY) { - credit_watchonly = txo.GetTxOut().nValue; } else { // We shouldn't see any other isminetypes Assume(false); @@ -278,13 +275,10 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) // Set the amounts in the return object if (wallet.IsTxImmatureCoinBase(wtx) && wtx.isConfirmed()) { ret.m_mine_immature += credit_mine; - ret.m_watchonly_immature += credit_watchonly; } else if (is_trusted && tx_depth >= min_depth) { ret.m_mine_trusted += credit_mine; - ret.m_watchonly_trusted += credit_watchonly; } else if (!is_trusted && wtx.InMempool()) { ret.m_mine_untrusted_pending += credit_mine; - ret.m_watchonly_untrusted_pending += credit_watchonly; } } } diff --git a/src/wallet/receive.h b/src/wallet/receive.h index 330fbf21a65..b995e21c959 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -49,9 +49,6 @@ struct Balance { CAmount m_mine_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more CAmount m_mine_untrusted_pending{0}; //!< Untrusted, but in mempool (pending) CAmount m_mine_immature{0}; //!< Immature coinbases in the main chain - CAmount m_watchonly_trusted{0}; - CAmount m_watchonly_untrusted_pending{0}; - CAmount m_watchonly_immature{0}; }; Balance GetBalance(const CWallet& wallet, int min_depth = 0, bool avoid_reuse = true); diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 16d93de1216..29da5b60eb5 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -171,7 +171,7 @@ RPCHelpMan getbalance() { {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Remains for backward compatibility. Must be excluded or set to \"*\"."}, {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "Only include transactions confirmed at least this many times."}, - {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also include balance in watch-only addresses (see 'importaddress')"}, + {"include_watchonly", RPCArg::Type::BOOL, RPCArg::Default{false}, "No longer used"}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{true}, "(only available if avoid_reuse wallet flag is set) Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."}, }, RPCResult{ @@ -203,13 +203,11 @@ RPCHelpMan getbalance() const auto min_depth{self.Arg("minconf")}; - bool include_watchonly = ParseIncludeWatchonly(request.params[2], *pwallet); - bool avoid_reuse = GetAvoidReuseFlag(*pwallet, request.params[3]); const auto bal = GetBalance(*pwallet, min_depth, avoid_reuse); - return ValueFromAmount(bal.m_mine_trusted + (include_watchonly ? bal.m_watchonly_trusted : 0)); + return ValueFromAmount(bal.m_mine_trusted); }, }; } diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 91f58efc622..8b51b0442e3 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -145,14 +145,10 @@ class WalletTest(BitcoinTestFramework): # getbalances expected_balances_0 = {'mine': {'immature': Decimal('0E-8'), 'trusted': Decimal('9.99'), # change from node 0's send - 'untrusted_pending': Decimal('60.0')}, - 'watchonly': {'immature': Decimal('5000'), - 'trusted': Decimal('50.0'), - 'untrusted_pending': Decimal('0E-8')}} + 'untrusted_pending': Decimal('60.0')}} expected_balances_1 = {'mine': {'immature': Decimal('0E-8'), 'trusted': Decimal('0E-8'), # node 1's send had an unsafe input 'untrusted_pending': Decimal('30.0') - fee_node_1}} # Doesn't include output of node 0's send since it was spent - del expected_balances_0["watchonly"] balances_0 = self.nodes[0].getbalances() balances_1 = self.nodes[1].getbalances() # remove lastprocessedblock keys (they will be tested later) From 1337c72198a7d32935431d64e9e58c12f9003abc Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 15 May 2025 15:47:47 -0700 Subject: [PATCH 4/7] wallet, rpc: Remove watchonly from RPCs Descriptor wallets don't have a conception of watchonly within a wallet, so remove all of these options and results from the RPCs --- src/wallet/rpc/addresses.cpp | 6 +- src/wallet/rpc/spend.cpp | 226 +++++++++---------- src/wallet/rpc/transactions.cpp | 49 +--- src/wallet/rpc/util.cpp | 15 -- src/wallet/rpc/util.h | 1 - src/wallet/rpc/wallet.cpp | 17 +- test/functional/rpc_psbt.py | 2 +- test/functional/wallet_balance.py | 4 +- test/functional/wallet_basic.py | 1 - test/functional/wallet_fundrawtransaction.py | 2 +- test/functional/wallet_importprunedfunds.py | 14 +- test/functional/wallet_listreceivedby.py | 6 +- test/functional/wallet_migration.py | 10 +- test/functional/wallet_send.py | 24 +- 14 files changed, 135 insertions(+), 242 deletions(-) diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index eece0815111..25c88a20427 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -380,7 +380,7 @@ RPCHelpMan getaddressinfo() {RPCResult::Type::STR, "address", "The bitcoin address validated."}, {RPCResult::Type::STR_HEX, "scriptPubKey", "The hex-encoded output script generated by the address."}, {RPCResult::Type::BOOL, "ismine", "If the address is yours."}, - {RPCResult::Type::BOOL, "iswatchonly", "If the address is watchonly."}, + {RPCResult::Type::BOOL, "iswatchonly", "(DEPRECATED) Always false."}, {RPCResult::Type::BOOL, "solvable", "If we know how to spend coins sent to this address, ignoring the possible lack of private keys."}, {RPCResult::Type::STR, "desc", /*optional=*/true, "A descriptor for spending coins sent to this address (only when solvable)."}, {RPCResult::Type::STR, "parent_desc", /*optional=*/true, "The descriptor used to derive this address if this is a descriptor wallet"}, @@ -402,7 +402,7 @@ RPCHelpMan getaddressinfo() {RPCResult::Type::OBJ, "embedded", /*optional=*/true, "Information about the address embedded in P2SH or P2WSH, if relevant and known.", { {RPCResult::Type::ELISION, "", "Includes all getaddressinfo output fields for the embedded address, excluding metadata (timestamp, hdkeypath, hdseedid)\n" - "and relation to the wallet (ismine, iswatchonly)."}, + "and relation to the wallet (ismine)."}, }}, {RPCResult::Type::BOOL, "iscompressed", /*optional=*/true, "If the pubkey is compressed."}, {RPCResult::Type::NUM_TIME, "timestamp", /*optional=*/true, "The creation time of the key, if available, expressed in " + UNIX_EPOCH_TIME + "."}, @@ -475,7 +475,7 @@ RPCHelpMan getaddressinfo() } } - ret.pushKV("iswatchonly", bool(mine & ISMINE_WATCH_ONLY)); + ret.pushKV("iswatchonly", false); UniValue detail = DescribeWalletAddress(*pwallet, dest); ret.pushKVs(std::move(detail)); diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 27bcbc3b94c..4e6fc63c486 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -146,9 +146,6 @@ static void PreventOutdatedOptions(const UniValue& options) if (options.exists("changePosition")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_position instead of changePosition"); } - if (options.exists("includeWatching")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Use include_watching instead of includeWatching"); - } if (options.exists("lockUnspents")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Use lock_unspents instead of lockUnspents"); } @@ -516,126 +513,118 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact std::optional change_position; bool lockUnspents = false; if (!options.isNull()) { - if (options.type() == UniValue::VBOOL) { - // backward compatibility bool only fallback - coinControl.fAllowWatchOnly = options.get_bool(); - } - else { - RPCTypeCheckObj(options, - { - {"add_inputs", UniValueType(UniValue::VBOOL)}, - {"include_unsafe", UniValueType(UniValue::VBOOL)}, - {"add_to_wallet", UniValueType(UniValue::VBOOL)}, - {"changeAddress", UniValueType(UniValue::VSTR)}, - {"change_address", UniValueType(UniValue::VSTR)}, - {"changePosition", UniValueType(UniValue::VNUM)}, - {"change_position", UniValueType(UniValue::VNUM)}, - {"change_type", UniValueType(UniValue::VSTR)}, - {"includeWatching", UniValueType(UniValue::VBOOL)}, - {"include_watching", UniValueType(UniValue::VBOOL)}, - {"inputs", UniValueType(UniValue::VARR)}, - {"lockUnspents", UniValueType(UniValue::VBOOL)}, - {"lock_unspents", UniValueType(UniValue::VBOOL)}, - {"locktime", UniValueType(UniValue::VNUM)}, - {"fee_rate", UniValueType()}, // will be checked by AmountFromValue() in SetFeeEstimateMode() - {"feeRate", UniValueType()}, // will be checked by AmountFromValue() below - {"psbt", UniValueType(UniValue::VBOOL)}, - {"solving_data", UniValueType(UniValue::VOBJ)}, - {"subtractFeeFromOutputs", UniValueType(UniValue::VARR)}, - {"subtract_fee_from_outputs", UniValueType(UniValue::VARR)}, - {"replaceable", UniValueType(UniValue::VBOOL)}, - {"conf_target", UniValueType(UniValue::VNUM)}, - {"estimate_mode", UniValueType(UniValue::VSTR)}, - {"minconf", UniValueType(UniValue::VNUM)}, - {"maxconf", UniValueType(UniValue::VNUM)}, - {"input_weights", UniValueType(UniValue::VARR)}, - {"max_tx_weight", UniValueType(UniValue::VNUM)}, - }, - true, true); + if (options.type() == UniValue::VBOOL) { + // backward compatibility bool only fallback, does nothing + } else { + RPCTypeCheckObj(options, + { + {"add_inputs", UniValueType(UniValue::VBOOL)}, + {"include_unsafe", UniValueType(UniValue::VBOOL)}, + {"add_to_wallet", UniValueType(UniValue::VBOOL)}, + {"changeAddress", UniValueType(UniValue::VSTR)}, + {"change_address", UniValueType(UniValue::VSTR)}, + {"changePosition", UniValueType(UniValue::VNUM)}, + {"change_position", UniValueType(UniValue::VNUM)}, + {"change_type", UniValueType(UniValue::VSTR)}, + {"includeWatching", UniValueType(UniValue::VBOOL)}, + {"include_watching", UniValueType(UniValue::VBOOL)}, + {"inputs", UniValueType(UniValue::VARR)}, + {"lockUnspents", UniValueType(UniValue::VBOOL)}, + {"lock_unspents", UniValueType(UniValue::VBOOL)}, + {"locktime", UniValueType(UniValue::VNUM)}, + {"fee_rate", UniValueType()}, // will be checked by AmountFromValue() in SetFeeEstimateMode() + {"feeRate", UniValueType()}, // will be checked by AmountFromValue() below + {"psbt", UniValueType(UniValue::VBOOL)}, + {"solving_data", UniValueType(UniValue::VOBJ)}, + {"subtractFeeFromOutputs", UniValueType(UniValue::VARR)}, + {"subtract_fee_from_outputs", UniValueType(UniValue::VARR)}, + {"replaceable", UniValueType(UniValue::VBOOL)}, + {"conf_target", UniValueType(UniValue::VNUM)}, + {"estimate_mode", UniValueType(UniValue::VSTR)}, + {"minconf", UniValueType(UniValue::VNUM)}, + {"maxconf", UniValueType(UniValue::VNUM)}, + {"input_weights", UniValueType(UniValue::VARR)}, + {"max_tx_weight", UniValueType(UniValue::VNUM)}, + }, + true, true); - if (options.exists("add_inputs")) { - coinControl.m_allow_other_inputs = options["add_inputs"].get_bool(); - } - - if (options.exists("changeAddress") || options.exists("change_address")) { - const std::string change_address_str = (options.exists("change_address") ? options["change_address"] : options["changeAddress"]).get_str(); - CTxDestination dest = DecodeDestination(change_address_str); - - if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Change address must be a valid bitcoin address"); + if (options.exists("add_inputs")) { + coinControl.m_allow_other_inputs = options["add_inputs"].get_bool(); } - coinControl.destChange = dest; - } - - if (options.exists("changePosition") || options.exists("change_position")) { - int pos = (options.exists("change_position") ? options["change_position"] : options["changePosition"]).getInt(); - if (pos < 0 || (unsigned int)pos > recipients.size()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds"); - } - change_position = (unsigned int)pos; - } - - if (options.exists("change_type")) { if (options.exists("changeAddress") || options.exists("change_address")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both change address and address type options"); + const std::string change_address_str = (options.exists("change_address") ? options["change_address"] : options["changeAddress"]).get_str(); + CTxDestination dest = DecodeDestination(change_address_str); + + if (!IsValidDestination(dest)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Change address must be a valid bitcoin address"); + } + + coinControl.destChange = dest; } - if (std::optional parsed = ParseOutputType(options["change_type"].get_str())) { - coinControl.m_change_type.emplace(parsed.value()); - } else { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown change type '%s'", options["change_type"].get_str())); + + if (options.exists("changePosition") || options.exists("change_position")) { + int pos = (options.exists("change_position") ? options["change_position"] : options["changePosition"]).getInt(); + if (pos < 0 || (unsigned int)pos > recipients.size()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds"); + } + change_position = (unsigned int)pos; } - } - const UniValue include_watching_option = options.exists("include_watching") ? options["include_watching"] : options["includeWatching"]; - coinControl.fAllowWatchOnly = ParseIncludeWatchonly(include_watching_option, wallet); - - if (options.exists("lockUnspents") || options.exists("lock_unspents")) { - lockUnspents = (options.exists("lock_unspents") ? options["lock_unspents"] : options["lockUnspents"]).get_bool(); - } - - if (options.exists("include_unsafe")) { - coinControl.m_include_unsafe_inputs = options["include_unsafe"].get_bool(); - } - - if (options.exists("feeRate")) { - if (options.exists("fee_rate")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both fee_rate (" + CURRENCY_ATOM + "/vB) and feeRate (" + CURRENCY_UNIT + "/kvB)"); + if (options.exists("change_type")) { + if (options.exists("changeAddress") || options.exists("change_address")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both change address and address type options"); + } + if (std::optional parsed = ParseOutputType(options["change_type"].get_str())) { + coinControl.m_change_type.emplace(parsed.value()); + } else { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown change type '%s'", options["change_type"].get_str())); + } } - if (options.exists("conf_target")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate."); + + if (options.exists("lockUnspents") || options.exists("lock_unspents")) { + lockUnspents = (options.exists("lock_unspents") ? options["lock_unspents"] : options["lockUnspents"]).get_bool(); } - if (options.exists("estimate_mode")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and feeRate"); + + if (options.exists("include_unsafe")) { + coinControl.m_include_unsafe_inputs = options["include_unsafe"].get_bool(); } - coinControl.m_feerate = CFeeRate(AmountFromValue(options["feeRate"])); - coinControl.fOverrideFeeRate = true; - } - if (options.exists("replaceable")) { - coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); - } - - if (options.exists("minconf")) { - coinControl.m_min_depth = options["minconf"].getInt(); - - if (coinControl.m_min_depth < 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative minconf"); + if (options.exists("feeRate")) { + if (options.exists("fee_rate")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both fee_rate (" + CURRENCY_ATOM + "/vB) and feeRate (" + CURRENCY_UNIT + "/kvB)"); + } + if (options.exists("conf_target")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate."); + } + if (options.exists("estimate_mode")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and feeRate"); + } + coinControl.m_feerate = CFeeRate(AmountFromValue(options["feeRate"])); + coinControl.fOverrideFeeRate = true; } - } - if (options.exists("maxconf")) { - coinControl.m_max_depth = options["maxconf"].getInt(); - - if (coinControl.m_max_depth < coinControl.m_min_depth) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("maxconf can't be lower than minconf: %d < %d", coinControl.m_max_depth, coinControl.m_min_depth)); + if (options.exists("replaceable")) { + coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); } + + if (options.exists("minconf")) { + coinControl.m_min_depth = options["minconf"].getInt(); + + if (coinControl.m_min_depth < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative minconf"); + } + } + + if (options.exists("maxconf")) { + coinControl.m_max_depth = options["maxconf"].getInt(); + + if (coinControl.m_max_depth < coinControl.m_min_depth) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("maxconf can't be lower than minconf: %d < %d", coinControl.m_max_depth, coinControl.m_min_depth)); + } + } + SetFeeEstimateMode(wallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee); } - SetFeeEstimateMode(wallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee); - } - } else { - // if options is null and not a bool - coinControl.fAllowWatchOnly = ParseIncludeWatchonly(NullUniValue, wallet); } if (options.exists("solving_data")) { @@ -757,13 +746,12 @@ RPCHelpMan fundrawtransaction() "Note that all inputs selected must be of standard form and P2SH scripts must be\n" "in the wallet using importdescriptors (to calculate fees).\n" "You can see whether this is the case by checking the \"solvable\" field in the listunspent output.\n" - "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only.\n" "Note that if specifying an exact fee rate, the resulting transaction may have a higher fee rate\n" "if the transaction has unconfirmed inputs. This is because the wallet will attempt to make the\n" "entire package have the given fee rate, not the resulting transaction.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "For backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", Cat>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{true}, "For a transaction with existing inputs, automatically include more if they are not enough."}, @@ -775,9 +763,7 @@ RPCHelpMan fundrawtransaction() {"changeAddress", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."}, - {"includeWatching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"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 'importdescriptors'."}, + {"includeWatching", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, {"lockUnspents", RPCArg::Type::BOOL, RPCArg::Default{false}, "Lock selected unspent outputs"}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, {"feeRate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_UNIT + "/kvB."}, @@ -1240,9 +1226,7 @@ RPCHelpMan send() {"change_position", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if change_address is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\" and \"bech32m\"."}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB.", RPCArgOptions{.also_positional = true}}, - {"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"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 'importdescriptors'."}, + {"include_watching", RPCArg::Type::BOOL, RPCArg::Default{"false"}, "(DEPRECATED) No longer used"}, {"inputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Specify inputs instead of adding them automatically.", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -1360,9 +1344,7 @@ RPCHelpMan sendall() { {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns the serialized transaction without broadcasting or adding it to the wallet"}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB.", RPCArgOptions{.also_positional = true}}, - {"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"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 'importdescriptors'."}, + {"include_watching", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, {"inputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Use exactly the specified inputs to build the transaction. Specifying inputs is incompatible with the send_max, minconf, and maxconf options.", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", @@ -1443,8 +1425,6 @@ RPCHelpMan sendall() SetFeeEstimateMode(*pwallet, coin_control, options["conf_target"], options["estimate_mode"], options["fee_rate"], /*override_min_fee=*/false); - coin_control.fAllowWatchOnly = ParseIncludeWatchonly(options["include_watching"], *pwallet); - if (options.exists("minconf")) { if (options["minconf"].getInt() < 0) { @@ -1491,7 +1471,7 @@ RPCHelpMan sendall() throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not available. UTXO (%s:%d) was already spent.", input.prevout.hash.ToString(), input.prevout.n)); } const CWalletTx* tx{pwallet->GetWalletTx(input.prevout.hash)}; - if (!tx || input.prevout.n >= tx->tx->vout.size() || !(pwallet->IsMine(tx->tx->vout[input.prevout.n]) & (coin_control.fAllowWatchOnly ? ISMINE_ALL : ISMINE_SPENDABLE))) { + if (!tx || input.prevout.n >= tx->tx->vout.size() || !(pwallet->IsMine(tx->tx->vout[input.prevout.n]) & ISMINE_SPENDABLE)) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not found. UTXO (%s:%d) is not part of wallet.", input.prevout.hash.ToString(), input.prevout.n)); } total_input_value += tx->tx->vout[input.prevout.n].nValue; @@ -1720,7 +1700,7 @@ RPCHelpMan walletcreatefundedpsbt() {"changeAddress", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."}, - {"includeWatching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch only"}, + {"includeWatching", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, {"lockUnspents", RPCArg::Type::BOOL, RPCArg::Default{false}, "Lock selected unspent outputs"}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, {"feeRate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_UNIT + "/kvB."}, diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 3580d2ceb06..69c5e01120b 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -68,7 +68,6 @@ struct tallyitem CAmount nAmount{0}; int nConf{std::numeric_limits::max()}; std::vector txids; - bool fIsWatchonly{false}; tallyitem() = default; }; @@ -86,10 +85,6 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons isminefilter filter = ISMINE_SPENDABLE; - if (ParseIncludeWatchonly(params[2], wallet)) { - filter |= ISMINE_WATCH_ONLY; - } - std::optional filtered_address{std::nullopt}; if (!by_label && !params[3].isNull() && !params[3].get_str().empty()) { if (!IsValidDestinationString(params[3].get_str())) { @@ -129,8 +124,6 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons item.nAmount += txout.nValue; item.nConf = std::min(item.nConf, nDepth); item.txids.push_back(wtx.GetHash()); - if (mine & ISMINE_WATCH_ONLY) - item.fIsWatchonly = true; } } @@ -147,21 +140,17 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons CAmount nAmount = 0; int nConf = std::numeric_limits::max(); - bool fIsWatchonly = false; if (it != mapTally.end()) { nAmount = (*it).second.nAmount; nConf = (*it).second.nConf; - fIsWatchonly = (*it).second.fIsWatchonly; } if (by_label) { tallyitem& _item = label_tally[label]; _item.nAmount += nAmount; _item.nConf = std::min(_item.nConf, nConf); - _item.fIsWatchonly = fIsWatchonly; } else { UniValue obj(UniValue::VOBJ); - if (fIsWatchonly) obj.pushKV("involvesWatchonly", true); obj.pushKV("address", EncodeDestination(address)); obj.pushKV("amount", ValueFromAmount(nAmount)); obj.pushKV("confirmations", (nConf == std::numeric_limits::max() ? 0 : nConf)); @@ -190,8 +179,6 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons CAmount nAmount = entry.second.nAmount; int nConf = entry.second.nConf; UniValue obj(UniValue::VOBJ); - if (entry.second.fIsWatchonly) - obj.pushKV("involvesWatchonly", true); obj.pushKV("amount", ValueFromAmount(nAmount)); obj.pushKV("confirmations", (nConf == std::numeric_limits::max() ? 0 : nConf)); obj.pushKV("label", entry.first); @@ -210,7 +197,7 @@ RPCHelpMan listreceivedbyaddress() { {"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "The minimum number of confirmations before payments are included."}, {"include_empty", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether to include addresses that haven't received any payments."}, - {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see 'importaddress')"}, + {"include_watchonly", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, {"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "If present and non-empty, only return information on this address."}, {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase transactions."}, }, @@ -219,7 +206,6 @@ RPCHelpMan listreceivedbyaddress() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR, "address", "The receiving address"}, {RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " received by the address"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included"}, @@ -264,7 +250,7 @@ RPCHelpMan listreceivedbylabel() { {"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "The minimum number of confirmations before payments are included."}, {"include_empty", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether to include labels that haven't received any payments."}, - {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see 'importaddress')"}, + {"include_watchonly", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase transactions."}, }, RPCResult{ @@ -272,7 +258,6 @@ RPCHelpMan listreceivedbylabel() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR_AMOUNT, "amount", "The total amount received by addresses with this label"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included"}, {RPCResult::Type::STR, "label", "The label of the receiving address. The default label is \"\""}, @@ -332,17 +317,12 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, filter_ismine, include_change); - bool involvesWatchonly = CachedTxIsFromMe(wallet, wtx, ISMINE_WATCH_ONLY); - // Sent if (!filter_label.has_value()) { for (const COutputEntry& s : listSent) { UniValue entry(UniValue::VOBJ); - if (involvesWatchonly || (wallet.IsMine(s.destination) & ISMINE_WATCH_ONLY)) { - entry.pushKV("involvesWatchonly", true); - } MaybePushAddress(entry, s.destination); entry.pushKV("category", "send"); entry.pushKV("amount", ValueFromAmount(-s.amount)); @@ -372,9 +352,6 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM continue; } UniValue entry(UniValue::VOBJ); - if (involvesWatchonly || (wallet.IsMine(r.destination) & ISMINE_WATCH_ONLY)) { - entry.pushKV("involvesWatchonly", true); - } MaybePushAddress(entry, r.destination); PushParentDescriptors(wallet, wtx.tx->vout.at(r.vout).scriptPubKey, entry); if (wtx.IsCoinBase()) @@ -450,14 +427,13 @@ RPCHelpMan listtransactions() "with the specified label, or \"*\" to disable filtering and return all transactions."}, {"count", RPCArg::Type::NUM, RPCArg::Default{10}, "The number of transactions to return"}, {"skip", RPCArg::Type::NUM, RPCArg::Default{0}, "The number of transactions to skip"}, - {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Include transactions to watch-only addresses (see 'importaddress')"}, + {"include_watchonly", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, }, RPCResult{ RPCResult::Type::ARR, "", "", { {RPCResult::Type::OBJ, "", "", Cat(Cat>( { - {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction."}, {RPCResult::Type::STR, "address", /*optional=*/true, "The bitcoin address of the transaction (not returned if the output does not have an address, e.g. OP_RETURN null data)."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" @@ -510,10 +486,6 @@ RPCHelpMan listtransactions() nFrom = request.params[2].getInt(); isminefilter filter = ISMINE_SPENDABLE; - if (ParseIncludeWatchonly(request.params[3], *pwallet)) { - filter |= ISMINE_WATCH_ONLY; - } - if (nCount < 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative count"); if (nFrom < 0) @@ -559,7 +531,7 @@ RPCHelpMan listsinceblock() { {"blockhash", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "If set, the block hash to list transactions since, otherwise list all transactions."}, {"target_confirmations", RPCArg::Type::NUM, RPCArg::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, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Include transactions to watch-only addresses (see 'importaddress')"}, + {"include_watchonly", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, {"include_removed", RPCArg::Type::BOOL, RPCArg::Default{true}, "Show transactions that were removed due to a reorg in the \"removed\" array\n" "(not guaranteed to work on pruned nodes)"}, {"include_change", RPCArg::Type::BOOL, RPCArg::Default{false}, "Also add entries for change outputs.\n"}, @@ -572,7 +544,6 @@ RPCHelpMan listsinceblock() { {RPCResult::Type::OBJ, "", "", Cat(Cat>( { - {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction."}, {RPCResult::Type::STR, "address", /*optional=*/true, "The bitcoin address of the transaction (not returned if the output does not have an address, e.g. OP_RETURN null data)."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" @@ -638,10 +609,6 @@ RPCHelpMan listsinceblock() } } - if (ParseIncludeWatchonly(request.params[2], wallet)) { - filter |= ISMINE_WATCH_ONLY; - } - bool include_removed = (request.params[3].isNull() || request.params[3].get_bool()); bool include_change = (!request.params[4].isNull() && request.params[4].get_bool()); @@ -701,8 +668,7 @@ RPCHelpMan gettransaction() "Get detailed information about in-wallet transaction \n", { {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, - {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, - "Whether to include watch-only addresses in balance calculation and details[]"}, + {"include_watchonly", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, {"verbose", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether to include a `decoded` field containing the decoded transaction (equivalent to RPC decoderawtransaction)"}, }, @@ -719,7 +685,6 @@ RPCHelpMan gettransaction() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction."}, {RPCResult::Type::STR, "address", /*optional=*/true, "The bitcoin address involved in the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" @@ -767,10 +732,6 @@ RPCHelpMan gettransaction() isminefilter filter = ISMINE_SPENDABLE; - if (ParseIncludeWatchonly(request.params[1], *pwallet)) { - filter |= ISMINE_WATCH_ONLY; - } - bool verbose = request.params[2].isNull() ? false : request.params[2].get_bool(); UniValue entry(UniValue::VOBJ); diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 1d344f2a241..eaca74b6e65 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -29,21 +29,6 @@ bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param) { 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. - */ -bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet) -{ - if (include_watchonly.isNull()) { - // if include_watchonly isn't explicitly set, then check if we have a watchonly wallet - return wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); - } - - // otherwise return whatever include_watchonly was set to - return include_watchonly.get_bool(); -} - bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) { if (request.URI.starts_with(WALLET_ENDPOINT_BASE)) { diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h index d524e515219..a73fb177b3e 100644 --- a/src/wallet/rpc/util.h +++ b/src/wallet/rpc/util.h @@ -43,7 +43,6 @@ void EnsureWalletIsUnlocked(const CWallet&); WalletContext& EnsureWalletContext(const std::any& context); bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param); -bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet); std::string LabelFromValue(const UniValue& value); //! Fetch parent descriptors of this scriptPubKey. void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, UniValue& entry); diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 74ffd192aed..1c417695788 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -573,7 +573,7 @@ RPCHelpMan simulaterawtransaction() }, {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { - {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see RPC importaddress)"}, + {"include_watchonly", RPCArg::Type::BOOL, RPCArg::Default{false}, "(DEPRECATED) No longer used"}, }, }, }, @@ -595,22 +595,7 @@ RPCHelpMan simulaterawtransaction() LOCK(wallet.cs_wallet); - UniValue include_watchonly(UniValue::VNULL); - if (request.params[1].isObject()) { - UniValue options = request.params[1]; - RPCTypeCheckObj(options, - { - {"include_watchonly", UniValueType(UniValue::VBOOL)}, - }, - true, true); - - include_watchonly = options["include_watchonly"]; - } - isminefilter filter = ISMINE_SPENDABLE; - if (ParseIncludeWatchonly(include_watchonly, wallet)) { - filter |= ISMINE_WATCH_ONLY; - } const auto& txs = request.params[0].get_array(); CAmount changes{0}; diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 9527053a6f7..73f536131b0 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -828,7 +828,7 @@ class PSBTTest(BitcoinTestFramework): # Test that psbts with p2pkh outputs are created properly p2pkh = self.nodes[0].getnewaddress(address_type='legacy') - psbt = self.nodes[1].walletcreatefundedpsbt([], [{p2pkh : 1}], 0, {"includeWatching" : True}, True) + psbt = self.nodes[1].walletcreatefundedpsbt(inputs=[], outputs=[{p2pkh : 1}], bip32derivs=True) self.nodes[0].decodepsbt(psbt['psbt']) # Test decoding error: invalid base64 diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 8b51b0442e3..8c83f42ecfd 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -83,9 +83,9 @@ class WalletTest(BitcoinTestFramework): assert_equal(self.nodes[0].getbalance("*"), 50) assert_equal(self.nodes[0].getbalance("*", 1), 50) assert_equal(self.nodes[0].getbalance(minconf=1), 50) - assert_equal(self.nodes[0].getbalance(minconf=0, include_watchonly=True), 50) + assert_equal(self.nodes[0].getbalance(minconf=0), 50) assert_equal(self.nodes[0].getbalance("*", 1, True), 50) - assert_equal(self.nodes[1].getbalance(minconf=0, include_watchonly=True), 50) + assert_equal(self.nodes[1].getbalance(minconf=0), 50) # Send 40 BTC from 0 to 1 and 60 BTC from 1 to 0. txs = create_transactions(self.nodes[0], self.nodes[1].getnewaddress(), 40, [Decimal('0.01')]) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 23c1616f9c6..496ccc5f314 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -532,7 +532,6 @@ class WalletTest(BitcoinTestFramework): assert_equal(address_info['address'], "mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ") assert_equal(address_info["scriptPubKey"], "76a9144e3854046c7bd1594ac904e4793b6a45b36dea0988ac") assert not address_info["ismine"] - assert not address_info["iswatchonly"] assert not address_info["isscript"] assert not address_info["ischange"] diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 9c5ef057d86..bf11ecee951 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -781,7 +781,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.nodes[3].loadwallet('wwatch') wwatch = self.nodes[3].get_wallet_rpc('wwatch') w3 = self.nodes[3].get_wallet_rpc(self.default_wallet_name) - result = wwatch.fundrawtransaction(rawtx, includeWatching=True, changeAddress=w3.getrawchangeaddress(), subtractFeeFromOutputs=[0]) + result = wwatch.fundrawtransaction(rawtx, changeAddress=w3.getrawchangeaddress(), subtractFeeFromOutputs=[0]) res_dec = self.nodes[0].decoderawtransaction(result["hex"]) assert_equal(len(res_dec["vin"]), 1) assert res_dec["vin"][0]["txid"] == self.watchonly_utxo['txid'] diff --git a/test/functional/wallet_importprunedfunds.py b/test/functional/wallet_importprunedfunds.py index b7b2589f554..d45be4aa188 100755 --- a/test/functional/wallet_importprunedfunds.py +++ b/test/functional/wallet_importprunedfunds.py @@ -52,15 +52,12 @@ class ImportPrunedFundsTest(BitcoinTestFramework): # Address Test - before import address_info = self.nodes[1].getaddressinfo(address1) - assert_equal(address_info['iswatchonly'], False) assert_equal(address_info['ismine'], False) address_info = self.nodes[1].getaddressinfo(address2) - assert_equal(address_info['iswatchonly'], False) assert_equal(address_info['ismine'], False) address_info = self.nodes[1].getaddressinfo(address3) - assert_equal(address_info['iswatchonly'], False) assert_equal(address_info['ismine'], False) # Send funds to self @@ -92,7 +89,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework): wwatch = self.nodes[1].get_wallet_rpc('wwatch') wwatch.importdescriptors([{"desc": self.nodes[0].getaddressinfo(address2)["desc"], "timestamp": "now"}]) wwatch.importprunedfunds(rawtransaction=rawtxn2, txoutproof=proof2) - assert [tx for tx in wwatch.listtransactions(include_watchonly=True) if tx['txid'] == txnid2] + assert [tx for tx in wwatch.listtransactions() if tx['txid'] == txnid2] # Import with private key with no rescan w1 = self.nodes[1].get_wallet_rpc(self.default_wallet_name) @@ -104,24 +101,21 @@ class ImportPrunedFundsTest(BitcoinTestFramework): # Addresses Test - after import address_info = w1.getaddressinfo(address1) - assert_equal(address_info['iswatchonly'], False) assert_equal(address_info['ismine'], False) address_info = wwatch.getaddressinfo(address2) - assert_equal(address_info['iswatchonly'], False) assert_equal(address_info['ismine'], True) address_info = w1.getaddressinfo(address3) - assert_equal(address_info['iswatchonly'], False) assert_equal(address_info['ismine'], True) # Remove transactions assert_raises_rpc_error(-4, f'Transaction {txnid1} does not belong to this wallet', w1.removeprunedfunds, txnid1) - assert not [tx for tx in w1.listtransactions(include_watchonly=True) if tx['txid'] == txnid1] + assert not [tx for tx in w1.listtransactions() if tx['txid'] == txnid1] wwatch.removeprunedfunds(txnid2) - assert not [tx for tx in wwatch.listtransactions(include_watchonly=True) if tx['txid'] == txnid2] + assert not [tx for tx in wwatch.listtransactions() if tx['txid'] == txnid2] w1.removeprunedfunds(txnid3) - assert not [tx for tx in w1.listtransactions(include_watchonly=True) if tx['txid'] == txnid3] + assert not [tx for tx in w1.listtransactions() if tx['txid'] == txnid3] # Check various RPC parameter validation errors assert_raises_rpc_error(-22, "TX decode failed", w1.importprunedfunds, b'invalid tx'.hex(), proof1) diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py index d6219af17c1..91dc1a9528f 100755 --- a/test/functional/wallet_listreceivedby.py +++ b/test/functional/wallet_listreceivedby.py @@ -27,7 +27,7 @@ class ReceivedByTest(BitcoinTestFramework): def run_test(self): # save the number of coinbase reward addresses so far - num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True)) + num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True)) self.log.info("listreceivedbyaddress Test") @@ -67,7 +67,7 @@ class ReceivedByTest(BitcoinTestFramework): # Test Address filtering # Only on addr expected = {"address": addr, "label": "", "amount": Decimal("0.1"), "confirmations": 10, "txids": [txid, ]} - res = self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True, address_filter=addr) + res = self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, address_filter=addr) assert_array_result(res, {"address": addr}, expected) assert_equal(len(res), 1) # Test for regression on CLI calls with address string (#14173) @@ -75,7 +75,7 @@ class ReceivedByTest(BitcoinTestFramework): assert_array_result(cli_res, {"address": addr}, expected) assert_equal(len(cli_res), 1) # Error on invalid address - assert_raises_rpc_error(-4, "address_filter parameter was invalid", self.nodes[1].listreceivedbyaddress, minconf=0, include_empty=True, include_watchonly=True, address_filter="bamboozling") + assert_raises_rpc_error(-4, "address_filter parameter was invalid", self.nodes[1].listreceivedbyaddress, minconf=0, include_empty=True, address_filter="bamboozling") # Another address receive money res = self.nodes[1].listreceivedbyaddress(0, True, True) assert_equal(len(res), 2 + num_cb_reward_addresses) # Right now 2 entries diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 4491c0b33fb..06c45ef65bf 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -277,7 +277,6 @@ class WalletMigrationTest(BitcoinTestFramework): # Transaction to multisig is in multisig1_watchonly and not multisig1 _, multisig1 = self.migrate_and_get_rpc("multisig1") assert_equal(multisig1.getaddressinfo(addr1)["ismine"], False) - assert_equal(multisig1.getaddressinfo(addr1)["iswatchonly"], False) assert_equal(multisig1.getaddressinfo(addr1)["solvable"], False) assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", multisig1.gettransaction, txid) assert_equal(multisig1.getbalance(), 0) @@ -363,7 +362,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_watchonly_txid) assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_sent_watchonly_utxo['txid']) assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, sent_watchonly_txid) - assert_equal(len(imports0.listtransactions(include_watchonly=True)), 2) + assert_equal(len(imports0.listtransactions()), 2) imports0.gettransaction(received_txid) imports0.gettransaction(watchonly_spendable_txid) assert_equal(imports0.getbalance(), spendable_bal) @@ -859,18 +858,14 @@ class WalletMigrationTest(BitcoinTestFramework): # Both addresses should only appear in the watchonly wallet p2pkh_addr_info = wallet.getaddressinfo(p2pkh_addr) - assert_equal(p2pkh_addr_info["iswatchonly"], False) assert_equal(p2pkh_addr_info["ismine"], False) p2wpkh_addr_info = wallet.getaddressinfo(p2wpkh_addr) - assert_equal(p2wpkh_addr_info["iswatchonly"], False) assert_equal(p2wpkh_addr_info["ismine"], False) watchonly_wallet = self.master_node.get_wallet_rpc(migrate_info["watchonly_name"]) watchonly_p2pkh_addr_info = watchonly_wallet.getaddressinfo(p2pkh_addr) - assert_equal(watchonly_p2pkh_addr_info["iswatchonly"], False) assert_equal(watchonly_p2pkh_addr_info["ismine"], True) watchonly_p2wpkh_addr_info = watchonly_wallet.getaddressinfo(p2wpkh_addr) - assert_equal(watchonly_p2wpkh_addr_info["iswatchonly"], False) assert_equal(watchonly_p2wpkh_addr_info["ismine"], True) # There should only be raw or addr descriptors @@ -1090,7 +1085,6 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(wallet.getbalances()['mine']['trusted'], 5) addr_info = wallet.getaddressinfo(wsh_pkh_addr) assert_equal(addr_info["ismine"], True) - assert_equal(addr_info["iswatchonly"], False) assert_equal(addr_info["solvable"], True) wallet.unloadwallet() @@ -1202,7 +1196,6 @@ class WalletMigrationTest(BitcoinTestFramework): # information about the witnessScript comp_wsh_pkh_addr_info = wallet.getaddressinfo(comp_wsh_pkh_addr) assert_equal(comp_wsh_pkh_addr_info["ismine"], True) - assert_equal(comp_wsh_pkh_addr_info["iswatchonly"], False) assert "embedded" in comp_wsh_pkh_addr_info # After migration, the invalid addresses should still not be detected as ismine and not watchonly. @@ -1211,7 +1204,6 @@ class WalletMigrationTest(BitcoinTestFramework): for addr in invalid_addrs: addr_info = wallet.getaddressinfo(addr) assert_equal(addr_info["ismine"], False) - assert_equal(addr_info["iswatchonly"], False) assert "embedded" not in addr_info wallet.unloadwallet() diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 067df43e50a..35fa9205ea1 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -42,7 +42,7 @@ class WalletSendTest(BitcoinTestFramework): arg_conf_target=None, arg_estimate_mode=None, arg_fee_rate=None, conf_target=None, estimate_mode=None, fee_rate=None, add_to_wallet=None, psbt=None, inputs=None, add_inputs=None, include_unsafe=None, change_address=None, change_position=None, change_type=None, - include_watching=None, locktime=None, lock_unspents=None, replaceable=None, subtract_fee_from_outputs=None, + locktime=None, lock_unspents=None, replaceable=None, subtract_fee_from_outputs=None, expect_error=None, solving_data=None, minconf=None): assert_not_equal((amount is None), (data is None)) @@ -90,8 +90,6 @@ class WalletSendTest(BitcoinTestFramework): options["change_position"] = change_position if change_type is not None: options["change_type"] = change_type - if include_watching is not None: - options["include_watching"] = include_watching if locktime is not None: options["locktime"] = locktime if lock_unspents is not None: @@ -110,6 +108,11 @@ class WalletSendTest(BitcoinTestFramework): if len(options.keys()) == 0: options = None + expect_sign = from_wallet.getwalletinfo()["private_keys_enabled"] + expect_sign = expect_sign and solving_data is None + if inputs is not None: + expect_sign = expect_sign and all(["weight" not in i for i in inputs]) + if expect_error is None: res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, fee_rate=arg_fee_rate, options=options) else: @@ -142,7 +145,7 @@ class WalletSendTest(BitcoinTestFramework): if locktime: return res - if from_wallet.getwalletinfo()["private_keys_enabled"] and not include_watching: + if expect_sign: assert_equal(res["complete"], True) assert "txid" in res else: @@ -154,7 +157,7 @@ class WalletSendTest(BitcoinTestFramework): if include_unsafe: from_balance += from_wallet.getbalances()["mine"]["untrusted_pending"] - if add_to_wallet and not include_watching: + if add_to_wallet: # Ensure transaction exists in the wallet: tx = from_wallet.gettransaction(res["txid"]) assert tx @@ -215,17 +218,13 @@ class WalletSendTest(BitcoinTestFramework): "desc": descsum_create("wpkh(" + xpub + "/0/0/*)"), "timestamp": "now", "range": [0, 100], - "keypool": True, "active": True, - "watchonly": True },{ "desc": descsum_create("wpkh(" + xpub + "/0/1/*)"), "timestamp": "now", "range": [0, 100], - "keypool": True, "active": True, "internal": True, - "watchonly": True }]) assert_equal(res, [{"success": True}, {"success": True}]) @@ -469,15 +468,15 @@ class WalletSendTest(BitcoinTestFramework): ext_utxo = ext_fund.listunspent(addresses=[addr])[0] # An external input without solving data should result in an error - self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, expect_error=(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"]))) + self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, expect_error=(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"]))) # But funding should work when the solving data is provided - res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"], addr_info["embedded"]["embedded"]["scriptPubKey"]]}) + res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"], addr_info["embedded"]["embedded"]["scriptPubKey"]]}) signed = ext_wallet.walletprocesspsbt(res["psbt"]) signed = ext_fund.walletprocesspsbt(res["psbt"]) assert signed["complete"] - res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"descriptors": [desc]}) + res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, solving_data={"descriptors": [desc]}) signed = ext_wallet.walletprocesspsbt(res["psbt"]) signed = ext_fund.walletprocesspsbt(res["psbt"]) assert signed["complete"] @@ -510,7 +509,6 @@ class WalletSendTest(BitcoinTestFramework): inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": input_weight}], add_inputs=True, psbt=True, - include_watching=True, fee_rate=target_fee_rate_sat_vb ) signed = ext_wallet.walletprocesspsbt(res["psbt"]) From 4439bf4b41a6997d4d965f00a8c40efa9cf6895b Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 15 May 2025 15:51:34 -0700 Subject: [PATCH 5/7] wallet, spend: Remove fWatchOnly from CCoinControl Descriptor wallets do not have a concept of watchonly within a wallet, so remove the watchonly option from coin control. --- src/wallet/coincontrol.h | 2 -- src/wallet/rpc/spend.cpp | 1 - src/wallet/spend.cpp | 4 ++-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index d36314312ad..d7075bc8d6e 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -89,8 +89,6 @@ public: //! If true, the selection process can add extra unselected inputs from the wallet //! while requires all selected inputs be used bool m_allow_other_inputs = true; - //! Includes watch only addresses which are solvable - bool fAllowWatchOnly = false; //! Override automatic min/max checks on fee, m_feerate must be set if true bool fOverrideFeeRate = false; //! Override the wallet's m_pay_tx_fee if set diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 4e6fc63c486..00f709c616a 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1067,7 +1067,6 @@ static RPCHelpMan bumpfee_helper(std::string method_name) Txid hash{Txid::FromUint256(ParseHashV(request.params[0], "txid"))}; CCoinControl coin_control; - coin_control.fAllowWatchOnly = pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); // optional parameters coin_control.m_signal_bip125_rbf = true; std::vector outputs; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index fe3f4b57b95..f054acb9bbd 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -54,7 +54,7 @@ static bool IsSegwit(const Descriptor& desc) { static bool UseMaxSig(const std::optional& txin, const CCoinControl* coin_control) { // Use max sig if watch only inputs were used or if this particular input is an external input // to ensure a sufficient fee is attained for the requested feerate. - return coin_control && (coin_control->fAllowWatchOnly || (txin && coin_control->IsExternalSelected(txin->prevout))); + return coin_control && txin && coin_control->IsExternalSelected(txin->prevout); } /** Get the size of an input (in witness units) once it's signed. @@ -429,7 +429,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, // Because CalculateMaximumSignedInputSize infers a solvable descriptor to get the satisfaction size, // it is safe to assume that this input is solvable if input_bytes is greater than -1. bool solvable = input_bytes > -1; - bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable)); + bool spendable = (mine & ISMINE_SPENDABLE) != ISMINE_NO; // Filter by spendable outputs only if (!spendable && params.only_spendable) continue; From 15710869e19e707ef03492c55030dcefa16269d8 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 15 May 2025 16:48:00 -0700 Subject: [PATCH 6/7] wallet: Remove ISMINE_WATCH_ONLY ISMINE_WATCH_ONLY has been removed from all places it was being used, and migration does not need ISMINE_WATCH_ONLY, so remove the enum. --- src/wallet/scriptpubkeyman.cpp | 1 - src/wallet/types.h | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 6a090c77334..730667dbdf4 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -204,7 +204,6 @@ isminetype LegacyDataSPKM::IsMine(const CScript& script) const case IsMineResult::NO: return ISMINE_NO; case IsMineResult::WATCH_ONLY: - return ISMINE_WATCH_ONLY; case IsMineResult::SPENDABLE: return ISMINE_SPENDABLE; } diff --git a/src/wallet/types.h b/src/wallet/types.h index 66eac17d45b..8d1202dd619 100644 --- a/src/wallet/types.h +++ b/src/wallet/types.h @@ -25,7 +25,6 @@ namespace wallet { * * For LegacyScriptPubKeyMan, * ISMINE_NO: the scriptPubKey is not in the wallet; - * ISMINE_WATCH_ONLY: the scriptPubKey has been imported into the wallet; * ISMINE_SPENDABLE: the scriptPubKey corresponds to an address owned by the wallet user (can spend with the private key); * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user; * ISMINE_ALL: all ISMINE flags except for USED; @@ -40,10 +39,9 @@ namespace wallet { */ enum isminetype : unsigned int { ISMINE_NO = 0, - ISMINE_WATCH_ONLY = 1 << 0, ISMINE_SPENDABLE = 1 << 1, ISMINE_USED = 1 << 2, - ISMINE_ALL = ISMINE_WATCH_ONLY | ISMINE_SPENDABLE, + ISMINE_ALL = ISMINE_SPENDABLE, ISMINE_ALL_USED = ISMINE_ALL | ISMINE_USED, ISMINE_ENUM_ELEMENTS, }; From b1a8ac07e91dd1d305fcbc16ea931d60e46c0055 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 26 May 2025 12:37:51 -0700 Subject: [PATCH 7/7] doc: Release note for removed watchonly parameters and results --- doc/release-notes-32618.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 doc/release-notes-32618.md diff --git a/doc/release-notes-32618.md b/doc/release-notes-32618.md new file mode 100644 index 00000000000..f231e3c1d5c --- /dev/null +++ b/doc/release-notes-32618.md @@ -0,0 +1,7 @@ +Wallet +------ + +* Since descriptor wallets do not allow mixing watchonly and non-watchonly descriptors, +the `include_watchonly` option (and its variants in naming) are removed from all RPCs +that had it. +* The `iswatchonly` field is removed from any RPCs that returned it.