Skip to content

Commit

Permalink
Merge #9662: Add createwallet "disableprivatekeys" option: a sane mod…
Browse files Browse the repository at this point in the history
…e for watchonly-wallets

Summary:
a3fa4d6a6acf19d640a1d5879a00aa1f059e2380 QA: Fix bug in -usecli logic that converts booleans to non-lowercase strings (Jonas Schnelli)
4704e5f074e57782d058404a594a7313cf170cf0 [QA] add createwallet disableprivatekey test (Jonas Schnelli)
c7b8f343e99d9d53ea353ddce9a977f1886caf30 [Qt] Disable creating receive addresses when private keys are disabled (Jonas Schnelli)
2f15c2bc20d583b4c1788da78c9c635c36e03ed0 Add disable privatekeys option to createwallet (Jonas Schnelli)
cebefba0855cee7fbcb9474b34e6779369e8e9ce Add option to disable private keys during internal wallet creation (Jonas Schnelli)
9995a602a639b64a749545b7c3bafbf67f97324f Add facility to store wallet flags (64 bits) (Jonas Schnelli)

Pull request description:

  This mode ('createwallet {"disableprivatekeys": true}') is intended for a sane pure watch-only mode, ideal for a use-case where one likes to use Bitcoin-Core in conjunction with a hardware-wallet or another solutions for cold-storage.

  Since we have support for custom change addresses in `fundrawtransaction`, pure watch-only wallets including coin-selection are possible and do make sense for some use cases.

  This new mode disables all forms of private key generation and ensure that no mix between hot and cold keys are possible.

Tree-SHA512: 3ebe7e8d54c4d4e5f790c348d4c292d456f573960a5b04d69ca5ef43a9217c7e7671761c6968cdc56f9a8bc235f3badd358576651af9f10855a0eb731f3fc508

Backport of Core PR9662
bitcoin/bitcoin#9662

Depends on D4175

Test Plan:
  make check
  test_runner.py
  bitcoin-qt
  Help -> Debug -> Console
  createwallet noprivkey true
Select noprivkey wallet in the main window
In the `Receive` menu, the `Request Payment` button should be greyed out and not clickable.
{F4088184}

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D4304
  • Loading branch information
jonspock committed Jul 17, 2020
1 parent 32f84a9 commit ce18076
Show file tree
Hide file tree
Showing 16 changed files with 299 additions and 31 deletions.
3 changes: 3 additions & 0 deletions src/interfaces/wallet.cpp
Expand Up @@ -423,6 +423,9 @@ namespace {
OutputType getDefaultAddressType() override {
return m_wallet.m_default_address_type;
}
bool IsWalletFlagSet(uint64_t flag) override {
return m_wallet.IsWalletFlagSet(flag);
}
OutputType getDefaultChangeType() override {
return m_wallet.m_default_change_type;
}
Expand Down
3 changes: 3 additions & 0 deletions src/interfaces/wallet.h
Expand Up @@ -210,6 +210,9 @@ class Wallet {
// Return whether HD enabled.
virtual bool hdEnabled() = 0;

// Check if a certain wallet flag is set.
virtual bool IsWalletFlagSet(uint64_t flag) = 0;

// Get default address type.
virtual OutputType getDefaultAddressType() = 0;

Expand Down
4 changes: 4 additions & 0 deletions src/qt/receivecoinsdialog.cpp
Expand Up @@ -105,6 +105,10 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model) {
// geometry is ready.
columnResizingFixer = new GUIUtil::TableViewLastColumnResizingFixer(
tableView, AMOUNT_MINIMUM_COLUMN_WIDTH, DATE_COLUMN_WIDTH, this);

// Eventually disable the main receive button if private key operations
// are disabled.
ui->receiveButton->setEnabled(!model->privateKeysDisabled());
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/qt/walletmodel.cpp
Expand Up @@ -445,6 +445,10 @@ bool WalletModel::isWalletEnabled() {
return !gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET);
}

bool WalletModel::privateKeysDisabled() const {
return m_wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
}

QString WalletModel::getWalletName() const {
return QString::fromStdString(m_wallet->getWalletName());
}
Expand Down
1 change: 1 addition & 0 deletions src/qt/walletmodel.h
Expand Up @@ -183,6 +183,7 @@ class WalletModel : public QObject {
const std::string &sRequest);

static bool isWalletEnabled();
bool privateKeysDisabled() const;

interfaces::Node &node() const { return m_node; }
interfaces::Wallet &wallet() const { return *m_wallet; }
Expand Down
1 change: 1 addition & 0 deletions src/rpc/client.cpp
Expand Up @@ -157,6 +157,7 @@ static const CRPCConvertParam vRPCConvertParams[] = {
{"echojson", 9, "arg9"},
{"rescanblockchain", 0, "start_height"},
{"rescanblockchain", 1, "stop_height"},
{"createwallet", 1, "disable_private_keys"},
};

class CRPCConvertTable {
Expand Down
39 changes: 34 additions & 5 deletions src/wallet/rpcwallet.cpp
Expand Up @@ -251,6 +251,11 @@ static UniValue getnewaddress(const Config &config,
HelpExampleRpc("getnewaddress", ""));
}

if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
throw JSONRPCError(RPC_WALLET_ERROR,
"Error: Private keys are disabled for this wallet");
}

LOCK2(cs_main, pwallet->cs_wallet);

// Parse the label first so we don't generate a key if there's an error
Expand Down Expand Up @@ -349,6 +354,11 @@ static UniValue getrawchangeaddress(const Config &config,
HelpExampleRpc("getrawchangeaddress", ""));
}

if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
throw JSONRPCError(RPC_WALLET_ERROR,
"Error: Private keys are disabled for this wallet");
}

LOCK2(cs_main, pwallet->cs_wallet);

if (!pwallet->IsLocked()) {
Expand Down Expand Up @@ -2861,7 +2871,12 @@ static UniValue keypoolrefill(const Config &config,
}
*/
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet);
if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
throw JSONRPCError(RPC_WALLET_ERROR,
"Error: Private keys are disabled for this wallet");
}

LOCK2(cs_main, pwallet->cs_wallet);

// 0 is interpreted by TopUpKeyPool() as the default keypool size given by
// -keypool
Expand Down Expand Up @@ -3390,6 +3405,9 @@ static UniValue getwalletinfo(const Config &config,
"/kB\n"
" \"hdchainid\": \"<hash160>\" (string) "
"the Hash160 of the HD master pubkey\n"
" \"private_keys_enabled\": true|false (boolean) false if "
"privatekeys are disabled for this wallet (enforced watch-only "
"wallet)\n"
"}\n"
"\nExamples:\n" +
HelpExampleCli("getwalletinfo", "") +
Expand Down Expand Up @@ -3437,6 +3455,8 @@ static UniValue getwalletinfo(const Config &config,
obj.push_back(Pair("hdaccounts", accounts));
obj.pushKV("unlocked_until", pwallet->nRelockTime);
obj.pushKV("paytxfee", ValueFromAmount(payTxFee.GetFeePerK()));
obj.pushKV("private_keys_enabled",
!pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
return obj;
}

Expand Down Expand Up @@ -3527,14 +3547,18 @@ static UniValue loadwallet(const Config &config,

static UniValue createwallet(const Config &config,
const JSONRPCRequest &request) {
if (request.fHelp || request.params.size() != 1) {
if (request.fHelp || request.params.size() < 1 ||
request.params.size() > 2) {
throw std::runtime_error(
"createwallet \"wallet_name\"\n"
"createwallet \"wallet_name\" ( disable_private_keys )\n"
"\nCreates and loads a new wallet.\n"
"\nArguments:\n"
"1. \"wallet_name\" (string, required) The name for the new wallet. "
"If this is a path, the wallet will be created at the path location.\n"
"2. \"password\" (string, optional) The password for the encrypted wallet or blank if empty. "
"3. disable_private_keys (boolean, optional, default: false) "
"Disable the possibility of private keys (only watchonlys are "
"possible in this mode).\n"
"\nResult:\n"
"{\n"
" \"name\" : <wallet_name>, (string) The wallet name if "
Expand All @@ -3554,6 +3578,10 @@ static UniValue createwallet(const Config &config,
std::string password = request.params[1].get_str();
std::string error;
std::string warning;
bool disable_privatekeys = false;
if (!request.params[1].isNull()) {
disable_privatekeys = request.params[1].get_bool();
}

WalletLocation location(wallet_name);

Expand All @@ -3576,7 +3604,8 @@ static UniValue createwallet(const Config &config,
std::shared_ptr<CWallet> const wallet = CWallet::CreateWalletFromFile(
chainParams,
*g_rpc_interfaces->chain,
location, passphrase, words, use_bls);
location, passphrase, words, use_bls,
(disable_privatekeys ? uint64_t(WALLET_FLAG_DISABLE_PRIVATE_KEYS) : 0));
if (!wallet) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet creation failed.");
}
Expand Down Expand Up @@ -4439,9 +4468,9 @@ static const ContextFreeRPCCommand commands[] = {
{ "wallet", "abandontransaction", abandontransaction, {"txid"} },
{ "wallet", "addmultisigaddress", addmultisigaddress, {"nrequired","keys","label|account"} },
{ "wallet", "backupwallet", backupwallet, {"destination"} },
{ "wallet", "createwallet", createwallet, {"wallet_name"} },
{ "wallet", "getlabeladdress", getlabeladdress, {"label"} },
{ "wallet", "getaddressesbylabels", getaddressesbylabels, {} },
{ "wallet", "createwallet", createwallet, {"wallet_name", "password", "disable_private_keys"} },
{ "wallet", "getbalance", getbalance, {"account","minconf","include_watchonly"} },
{ "wallet", "getnewaddress", getnewaddress, {"label|account"} },
{ "wallet", "getrawchangeaddress", getrawchangeaddress, {} },
Expand Down
12 changes: 0 additions & 12 deletions src/wallet/test/wallet_tests.cpp
Expand Up @@ -867,15 +867,3 @@ TEST_CASE("ListCoins") {
BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U);
}

BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) {
auto chain = interfaces::MakeChain();
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(
Params(), *chain, WalletLocation(), WalletDatabase::CreateDummy());
wallet->SetMinVersion(FEATURE_LATEST);
wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
BOOST_CHECK(!wallet->TopUpKeyPool(1000));
CPubKey pubkey;
BOOST_CHECK(!wallet->GetKeyFromPool(pubkey, false));
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit ce18076

Please sign in to comment.