From 9e9975f83b9635955f8d022004c6de584cc897eb Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Fri, 19 Feb 2021 11:37:57 +1300 Subject: [PATCH] Merge #21201: rpc: Disallow sendtoaddress and sendmany when private keys disabled 6bfbc97d716faad38c87603ac6049d222236d623 test: disallow sendtoaddress/sendmany when private keys disabled (Jon Atack) 0997019e7681efb00847a7246c15ac8f235128d8 Disallow sendtoaddress and sendmany when private keys disabled (Andrew Chow) Pull request description: Since `sendtoaddress` and `sendmany` (which use the `SendMoney` function) create and commit a transaction, they should not do anything when the wallet does not have private keys. Otherwise a valid transaction cannot be made. Fixes #21104 ACKs for top commit: jonatack: ACK 6bfbc97d716faad38c87603ac6049d222236d623 meshcollider: utACK 6bfbc97d716faad38c87603ac6049d222236d623 kristapsk: ACK 6bfbc97d716faad38c87603ac6049d222236d623. "Error: Private keys are disabled for this wallet" is definitely a better error message than "Insufficient funds" here. Hopefully change of error code from -6 to -4 doesn't break any software using Bitcoin JSON-RPC API. Tree-SHA512: f277d6b5252e43942d568614032596f2c0827f00cd0cb71e44ffcb9822bfb15a71730a3e3688f31e59ba4eb7d275250c4e65ad4b6b3e96be6314c56a672432fb --- src/wallet/rpcwallet.cpp | 8 +++++++- test/functional/wallet_watchonly.py | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 40ede2a706..79343ff868 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -382,6 +382,12 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto { EnsureWalletIsUnlocked(wallet); + // This function is only used by sendtoaddress and sendmany. + // This should always try to sign, if we don't have private keys, don't try to do anything here. + if (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); + } + if (coin_control.IsUsingCoinJoin()) { map_value["DS"] = "1"; } @@ -392,7 +398,7 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto bilingual_str error; CTransactionRef tx; FeeCalculation fee_calc_out; - bool fCreated = wallet.CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, !wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); + const bool fCreated = wallet.CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, true); if (!fCreated) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); } diff --git a/test/functional/wallet_watchonly.py b/test/functional/wallet_watchonly.py index 2b91edf22f..c3ac93229f 100755 --- a/test/functional/wallet_watchonly.py +++ b/test/functional/wallet_watchonly.py @@ -2,7 +2,7 @@ # 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. +"""Test createwallet watchonly arguments. """ from test_framework.blocktools import COINBASE_MATURITY @@ -51,6 +51,11 @@ class CreateWalletWatchonlyTest(BitcoinTestFramework): assert_equal(len(wo_wallet.listtransactions()), 1) assert_equal(wo_wallet.getbalance(include_watchonly=False), 0) + self.log.info('Test sending from a watch-only wallet raises RPC error') + msg = "Error: Private keys are disabled for this wallet" + assert_raises_rpc_error(-4, msg, wo_wallet.sendtoaddress, a1, 0.1) + assert_raises_rpc_error(-4, msg, wo_wallet.sendmany, amounts={a1: 0.1}) + self.log.info('Testing listreceivedbyaddress watch-only defaults') result = wo_wallet.listreceivedbyaddress() assert_equal(len(result), 1)