foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-04 13:55:23 -05:00
Code Issues Activity

Merge #15454: Remove the automatic creation and loading of the default wallet

Browse source 
d26f0648f1 Tell users how to load or create a wallet when no wallet is loaded (Andrew Chow)
1bee1e6269 Do not create default wallet (Andrew Chow)

Pull request description:

  Instead of automatically creating and loading a default wallet, users should instead explicitly create their wallet or load it on start.

  Builds on #19754 which provides the `load_on_startup` behavior for the GUI.

ACKs for top commit:
  jnewbery:
    Manual test and very light code review ACK d26f0648f1
  ryanofsky:
    Code review ACK d26f0648f1. Just suggested changes to first commit (reusing MakeWalletDatabase and adding release notes), no changes to second commit
  jonatack:
    ACK d26f0648f1 light code review, debug build, ran tests, did manual testing with testnet, rebased on master, on linux debian.

Tree-SHA512: 091d785aef64736f7df661c576e815a87f3d029cfa32f3a75ba86fc25795f10b022ab3ae15c5b61a10b8cee16f5650f15cd79cbd6127e5e3ccbef631966d3c30
This commit is contained in:
Samuel Dobson 2020-09-18 11:39:24 +12:00
commit 652c45fdbb
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
21 changed files with 96 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
doc/release-notes-15454.md Normal file
View file

@ -0,0 +1,6 @@
Wallet
------
Bitcoin Core will no longer create an unnamed `""` wallet by default when no wallet is specified on the command line or in the configuration files.
For backwards compatibility, if an unnamed `""` wallet already exists and would have been loaded previously, then it will still be loaded.
Users without an unnamed `""` wallet and without any other wallets to be loaded on startup will be prompted to either choose a wallet to load, or to create a new wallet.

11
src/interfaces/wallet.cpp
View file

@ -488,8 +488,7 @@ public:
class WalletClientImpl : public WalletClient
{
public:
WalletClientImpl(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames)
: m_wallet_filenames(std::move(wallet_filenames))
WalletClientImpl(Chain& chain, ArgsManager& args)
{
m_context.chain = &chain;
m_context.args = &args;
@ -506,8 +505,8 @@ public:
m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back()));
}
}
bool verify() override { return VerifyWallets(*m_context.chain, m_wallet_filenames); }
bool load() override { return LoadWallets(*m_context.chain, m_wallet_filenames); }
bool verify() override { return VerifyWallets(*m_context.chain); }
bool load() override { return LoadWallets(*m_context.chain); }
void start(CScheduler& scheduler) override { return StartWallets(scheduler, *Assert(m_context.args)); }
void flush() override { return FlushWallets(); }
void stop() override { return StopWallets(); }
@ -566,9 +565,9 @@ public:
std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<WalletImpl>(wallet) : nullptr; }
std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames)
std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args)
{
return MakeUnique<WalletClientImpl>(chain, args, std::move(wallet_filenames));
return MakeUnique<WalletClientImpl>(chain, args);
}
} // namespace interfaces

2
src/interfaces/wallet.h
View file

@ -411,7 +411,7 @@ std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet);
//! Return implementation of ChainClient interface for a wallet client. This
//! function will be undefined in builds where ENABLE_WALLET is false.
std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames);
std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args);
} // namespace interfaces

5
src/qt/bitcoingui.cpp
View file

@ -660,6 +660,11 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller)
}
}
WalletController* BitcoinGUI::getWalletController()
{
return m_wallet_controller;
}
void BitcoinGUI::addWallet(WalletModel* walletModel)
{
if (!walletFrame) return;

1
src/qt/bitcoingui.h
View file

@ -79,6 +79,7 @@ public:
void setClientModel(ClientModel *clientModel = nullptr, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);
#ifdef ENABLE_WALLET
void setWalletController(WalletController* wallet_controller);
WalletController* getWalletController();
#endif
#ifdef ENABLE_WALLET

25
src/qt/walletframe.cpp
View file

@ -2,6 +2,8 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <qt/createwalletdialog.h>
#include <qt/walletcontroller.h>
#include <qt/walletframe.h>
#include <qt/walletmodel.h>
@ -10,8 +12,11 @@
#include <cassert>
#include <QGroupBox>
#include <QHBoxLayout>
#include <QLabel>
#include <QPushButton>
#include <QVBoxLayout>
WalletFrame::WalletFrame(const PlatformStyle *_platformStyle, BitcoinGUI *_gui) :
QFrame(_gui),
@ -25,9 +30,25 @@ WalletFrame::WalletFrame(const PlatformStyle *_platformStyle, BitcoinGUI *_gui)
walletFrameLayout->setContentsMargins(0,0,0,0);
walletFrameLayout->addWidget(walletStack);
QLabel *noWallet = new QLabel(tr("No wallet has been loaded."));
// hbox for no wallet
QGroupBox* no_wallet_group = new QGroupBox(walletStack);
QVBoxLayout* no_wallet_layout = new QVBoxLayout(no_wallet_group);
QLabel *noWallet = new QLabel(tr("No wallet has been loaded.\nGo to File > Open Wallet to load a wallet.\n- OR -"));
noWallet->setAlignment(Qt::AlignCenter);
walletStack->addWidget(noWallet);
no_wallet_layout->addWidget(noWallet, 0, Qt::AlignHCenter | Qt::AlignBottom);
// A button for create wallet dialog
QPushButton* create_wallet_button = new QPushButton(tr("Create a new wallet"), walletStack);
connect(create_wallet_button, &QPushButton::clicked, [this] {
auto activity = new CreateWalletActivity(gui->getWalletController(), this);
connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater);
activity->create();
});
no_wallet_layout->addWidget(create_wallet_button, 0, Qt::AlignHCenter | Qt::AlignTop);
no_wallet_group->setLayout(no_wallet_layout);
walletStack->addWidget(no_wallet_group);
}
WalletFrame::~WalletFrame()

11
src/wallet/init.cpp
View file

@ -107,16 +107,7 @@ void WalletInit::Construct(NodeContext& node) const
LogPrintf("Wallet disabled!\n");
return;
}
// If there's no -wallet setting with a list of wallets to load, set it to
// load the default "" wallet.
if (!args.IsArgSet("wallet")) {
args.LockSettings([&](util::Settings& settings) {
util::SettingsValue wallets(util::SettingsValue::VARR);
wallets.push_back(""); // Default wallet name is ""
settings.rw_settings["wallet"] = wallets;
});
}
auto wallet_client = interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"));
auto wallet_client = interfaces::MakeWalletClient(*node.chain, args);
node.wallet_client = wallet_client.get();
node.chain_clients.emplace_back(std::move(wallet_client));
}

25
src/wallet/load.cpp
View file

@ -16,7 +16,7 @@
#include <univalue.h>
bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files)
bool VerifyWallets(interfaces::Chain& chain)
{
if (gArgs.IsArgSet("-walletdir")) {
fs::path wallet_dir = gArgs.GetArg("-walletdir", "");
@ -41,10 +41,27 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
chain.initMessage(_("Verifying wallet(s)...").translated);
// For backwards compatibility if an unnamed top level wallet exists in the
// wallets directory, include it in the default list of wallets to load.
if (!gArgs.IsArgSet("wallet")) {
DatabaseOptions options;
DatabaseStatus status;
bilingual_str error_string;
options.require_existing = true;
options.verify = false;
if (MakeWalletDatabase("", options, status, error_string)) {
gArgs.LockSettings([&](util::Settings& settings) {
util::SettingsValue wallets(util::SettingsValue::VARR);
wallets.push_back(""); // Default wallet name is ""
settings.rw_settings["wallet"] = wallets;
});
}
}
// Keep track of each wallet absolute path to detect duplicates.
std::set<fs::path> wallet_paths;
for (const auto& wallet_file : wallet_files) {
for (const auto& wallet_file : gArgs.GetArgs("-wallet")) {
const fs::path path = fs::absolute(wallet_file, GetWalletDir());
if (!wallet_paths.insert(path).second) {
@ -65,10 +82,10 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
return true;
}
bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files)
bool LoadWallets(interfaces::Chain& chain)
{
try {
for (const std::string& name : wallet_files) {
for (const std::string& name : gArgs.GetArgs("-wallet")) {
DatabaseOptions options;
DatabaseStatus status;
options.verify = false; // No need to verify, assuming verified earlier in VerifyWallets()

4
src/wallet/load.h
View file

@ -17,10 +17,10 @@ class Chain;
} // namespace interfaces
//! Responsible for reading and validating the -wallet arguments and verifying the wallet database.
bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files);
bool VerifyWallets(interfaces::Chain& chain);
//! Load wallet databases.
bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files);
bool LoadWallets(interfaces::Chain& chain);
//! Complete startup of wallets.
void StartWallets(CScheduler& scheduler, const ArgsManager& args);

2
src/wallet/test/init_test_fixture.cpp
View file

@ -10,7 +10,7 @@
InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName)
{
m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {});
m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args));
std::string sep;
sep += fs::path::preferred_separator;

2
src/wallet/test/wallet_test_fixture.h
View file

@ -21,7 +21,7 @@ struct WalletTestingSetup : public TestingSetup {
explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node);
std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {});
std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args));
CWallet m_wallet;
std::unique_ptr<interfaces::Handler> m_chain_notifications_handler;
};

4
test/functional/feature_backwards_compatibility.py
View file

@ -36,12 +36,12 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
self.num_nodes = 6
# Add new version after each release:
self.extra_args = [
["-addresstype=bech32"], # Pre-release: use to mine blocks
["-addresstype=bech32", "-wallet="], # Pre-release: use to mine blocks
["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # Pre-release: use to receive coins, swap wallets, etc
["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # v0.19.1
["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # v0.18.1
["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # v0.17.2
["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # v0.16.3
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-wallet=wallet.dat"], # v0.16.3
]
def skip_test_if_missing_module(self):

6
test/functional/feature_fee_estimation.py
View file

@ -145,9 +145,9 @@ class EstimateFeeTest(BitcoinTestFramework):
# mine non-standard txs (e.g. txs with "dust" outputs)
# Force fSendTrickle to true (via whitelist.noban)
self.extra_args = [
["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1"],
["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-blockmaxweight=68000"],
["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-blockmaxweight=32000"],
["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-wallet="],
["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-blockmaxweight=68000", "-wallet="],
["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-blockmaxweight=32000", "-wallet="],
]
def skip_test_if_missing_module(self):

4
test/functional/feature_filelock.py
View file

@ -15,7 +15,7 @@ class FilelockTest(BitcoinTestFramework):
def setup_network(self):
self.add_nodes(self.num_nodes, extra_args=None)
self.nodes[0].start([])
self.nodes[0].start(['-wallet='])
self.nodes[0].wait_for_rpc_connection()
def run_test(self):
@ -30,7 +30,7 @@ class FilelockTest(BitcoinTestFramework):
wallet_dir = os.path.join(datadir, 'wallets')
self.log.info("Check that we can't start a second bitcoind instance using the same wallet")
expected_msg = "Error: Error initializing wallet database environment"
self.nodes[1].assert_start_raises_init_error(extra_args=['-walletdir={}'.format(wallet_dir), '-noserver'], expected_msg=expected_msg, match=ErrorMatch.PARTIAL_REGEX)
self.nodes[1].assert_start_raises_init_error(extra_args=['-walletdir={}'.format(wallet_dir), '-wallet=', '-noserver'], expected_msg=expected_msg, match=ErrorMatch.PARTIAL_REGEX)
if __name__ == '__main__':
FilelockTest().main()

2
test/functional/mempool_compatibility.py
View file

@ -31,7 +31,7 @@ class MempoolCompatibilityTest(BitcoinTestFramework):
150200, # oldest version supported by the test framework
None,
])
self.start_nodes()
self.start_nodes([[], ["-wallet="]])
self.import_deterministic_coinbase_privkeys()
def run_test(self):

8
test/functional/wallet_backup.py
View file

@ -50,10 +50,10 @@ class WalletBackupTest(BitcoinTestFramework):
# nodes 1, 2,3 are spenders, let's give them a keypool=100
# whitelist all peers to speed up tx relay / mempool sync
self.extra_args = [
["-whitelist=noban@127.0.0.1", "-keypool=100"],
["-whitelist=noban@127.0.0.1", "-keypool=100"],
["-whitelist=noban@127.0.0.1", "-keypool=100"],
["-whitelist=noban@127.0.0.1"],
["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
["-whitelist=noban@127.0.0.1", "-wallet="],
]
self.rpc_timeout = 120

2
test/functional/wallet_dump.py
View file

@ -95,7 +95,7 @@ def read_dump(file_name, addrs, script_addrs, hd_master_addr_old):
class WalletDumpTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
self.extra_args = [["-keypool=90", "-addresstype=legacy"]]
self.extra_args = [["-keypool=90", "-addresstype=legacy", "-wallet=dump"]]
self.rpc_timeout = 120
def skip_test_if_missing_module(self):

4
test/functional/wallet_import_rescan.py
View file

@ -151,7 +151,7 @@ class ImportRescanTest(BitcoinTestFramework):
self.skip_if_no_wallet()
def setup_network(self):
self.extra_args = [[] for _ in range(self.num_nodes)]
self.extra_args = [["-wallet="] for _ in range(self.num_nodes)]
for i, import_node in enumerate(IMPORT_NODES, 2):
if import_node.prune:
self.extra_args[i] += ["-prune=1"]
@ -159,7 +159,7 @@ class ImportRescanTest(BitcoinTestFramework):
self.add_nodes(self.num_nodes, extra_args=self.extra_args)
# Import keys with pruning disabled
self.start_nodes(extra_args=[[]] * self.num_nodes)
self.start_nodes(extra_args=[["-wallet="]] * self.num_nodes)
for n in self.nodes:
n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase')
self.stop_nodes()

5
test/functional/wallet_multiwallet.py
View file

@ -43,6 +43,7 @@ class MultiWalletTest(BitcoinTestFramework):
self.setup_clean_chain = True
self.num_nodes = 2
self.rpc_timeout = 120
self.extra_args = [["-wallet="], ["-wallet="]]
def skip_test_if_missing_module(self):
self.skip_if_no_wallet()
@ -82,7 +83,7 @@ class MultiWalletTest(BitcoinTestFramework):
os.rename(wallet_dir("wallet.dat"), wallet_dir("w8"))
# create another dummy wallet for use in testing backups later
self.start_node(0, [])
self.start_node(0, ["-wallet="])
self.stop_nodes()
empty_wallet = os.path.join(self.options.tmpdir, 'empty.dat')
os.rename(wallet_dir("wallet.dat"), empty_wallet)
@ -152,7 +153,7 @@ class MultiWalletTest(BitcoinTestFramework):
competing_wallet_dir = os.path.join(self.options.tmpdir, 'competing_walletdir')
os.mkdir(competing_wallet_dir)
self.restart_node(0, ['-walletdir=' + competing_wallet_dir])
self.restart_node(0, ['-walletdir=' + competing_wallet_dir, '-wallet='])
exp_stderr = r"Error: Error initializing wallet database environment \"\S+competing_walletdir\"!"
self.nodes[1].assert_start_raises_init_error(['-walletdir=' + competing_wallet_dir], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)

10
test/functional/wallet_startup.py
View file

@ -26,6 +26,16 @@ class WalletStartupTest(BitcoinTestFramework):
self.start_nodes()
def run_test(self):
self.log.info('Should start without any wallets')
assert_equal(self.nodes[0].listwallets(), [])
assert_equal(self.nodes[0].listwalletdir(), {'wallets': []})
self.log.info('New default wallet should load by default when there are no other wallets')
self.nodes[0].createwallet(wallet_name='', load_on_startup=False)
self.restart_node(0)
assert_equal(self.nodes[0].listwallets(), [''])
self.log.info('Test load on startup behavior')
self.nodes[0].createwallet(wallet_name='w0', load_on_startup=True)
self.nodes[0].createwallet(wallet_name='w1', load_on_startup=False)
self.nodes[0].createwallet(wallet_name='w2', load_on_startup=True)

2
test/functional/wallet_upgradewallet.py
View file

@ -27,7 +27,7 @@ class UpgradeWalletTest(BitcoinTestFramework):
self.setup_clean_chain = True
self.num_nodes = 3
self.extra_args = [
["-addresstype=bech32"], # current wallet version
["-addresstype=bech32", "-wallet="], # current wallet version
["-usehd=1"], # v0.16.3 wallet
["-usehd=0"] # v0.15.2 wallet
]