Remove direct node->wallet calls in init.cpp

Route calls during node initialization and shutdown that would happen between a
node process and wallet processes through the serializable `Chain::Client`
interface, rather than `WalletInitInterface` which is now simpler and only
deals with early initialization and parameter interaction.

This commit mostly does not change behavior. The only change is that the
"Wallet disabled!" and "No wallet support compiled in!" messages are now logged
earlier during startup.
This commit is contained in:
Russell Yanofsky 2017-09-28 14:13:29 -04:00
parent 8db11dd0b1
commit ea961c3d72
11 changed files with 114 additions and 85 deletions

View file

@ -467,6 +467,7 @@ endif
bitcoind_LDADD = \
$(LIBBITCOIN_SERVER) \
$(LIBBITCOIN_WALLET) \
$(LIBBITCOIN_SERVER) \
$(LIBBITCOIN_COMMON) \
$(LIBUNIVALUE) \
$(LIBBITCOIN_UTIL) \

View file

@ -14,13 +14,7 @@ public:
bool HasWalletSupport() const override {return false;}
void AddWalletOptions() const override;
bool ParameterInteraction() const override {return true;}
void RegisterRPC(CRPCTable &) const override {}
bool Verify(interfaces::Chain& chain) const override {return true;}
bool Open(interfaces::Chain& chain) const override {LogPrintf("No wallet support compiled in!\n"); return true;}
void Start(CScheduler& scheduler) const override {}
void Flush() const override {}
void Stop() const override {}
void Close() const override {}
void Construct(InitInterfaces& interfaces) const override {LogPrintf("No wallet support compiled in!\n");}
};
void DummyWalletInit::AddWalletOptions() const

View file

@ -19,6 +19,7 @@
#include <fs.h>
#include <httpserver.h>
#include <httprpc.h>
#include <interfaces/chain.h>
#include <index/txindex.h>
#include <key.h>
#include <validation.h>
@ -177,7 +178,9 @@ void Shutdown(InitInterfaces& interfaces)
StopREST();
StopRPC();
StopHTTPServer();
g_wallet_init_interface.Flush();
for (const auto& client : interfaces.chain_clients) {
client->flush();
}
StopMapPort();
// Because these depend on each-other, we make sure that neither can be
@ -240,7 +243,9 @@ void Shutdown(InitInterfaces& interfaces)
pcoinsdbview.reset();
pblocktree.reset();
}
g_wallet_init_interface.Stop();
for (const auto& client : interfaces.chain_clients) {
client->stop();
}
#if ENABLE_ZMQ
if (g_zmq_notification_interface) {
@ -260,7 +265,7 @@ void Shutdown(InitInterfaces& interfaces)
UnregisterAllValidationInterfaces();
GetMainSignals().UnregisterBackgroundSignalScheduler();
GetMainSignals().UnregisterWithMempoolSignals(mempool);
g_wallet_init_interface.Close();
interfaces.chain_clients.clear();
globalVerifyHandle.reset();
ECC_Stop();
LogPrintf("%s: done\n", __func__);
@ -1222,11 +1227,19 @@ bool AppInitMain(InitInterfaces& interfaces)
GetMainSignals().RegisterBackgroundSignalScheduler(scheduler);
GetMainSignals().RegisterWithMempoolSignals(mempool);
// Create client interfaces for wallets that are supposed to be loaded
// according to -wallet and -disablewallet options. This only constructs
// the interfaces, it doesn't load wallet data. Wallets actually get loaded
// when load() and start() interface methods are called below.
g_wallet_init_interface.Construct(interfaces);
/* Register RPC commands regardless of -server setting so they will be
* available in the GUI RPC console even if external calls are disabled.
*/
RegisterAllCoreRPCCommands(tableRPC);
g_wallet_init_interface.RegisterRPC(tableRPC);
for (const auto& client : interfaces.chain_clients) {
client->registerRpcs();
}
g_rpc_interfaces = &interfaces;
#if ENABLE_ZMQ
RegisterZMQRPCCommands(tableRPC);
@ -1245,7 +1258,11 @@ bool AppInitMain(InitInterfaces& interfaces)
}
// ********************************************************* Step 5: verify wallet database integrity
if (!g_wallet_init_interface.Verify(*interfaces.chain)) return false;
for (const auto& client : interfaces.chain_clients) {
if (!client->verify()) {
return false;
}
}
// ********************************************************* Step 6: network initialization
// Note that we absolutely cannot open any actual connections
@ -1564,7 +1581,11 @@ bool AppInitMain(InitInterfaces& interfaces)
}
// ********************************************************* Step 9: load wallet
if (!g_wallet_init_interface.Open(*interfaces.chain)) return false;
for (const auto& client : interfaces.chain_clients) {
if (!client->load()) {
return false;
}
}
// ********************************************************* Step 10: data directory maintenance
@ -1710,7 +1731,9 @@ bool AppInitMain(InitInterfaces& interfaces)
SetRPCWarmupFinished();
uiInterface.InitMessage(_("Done loading"));
g_wallet_init_interface.Start(scheduler);
for (const auto& client : interfaces.chain_clients) {
client->start(scheduler);
}
return true;
}

View file

@ -9,6 +9,8 @@
#include <string>
#include <vector>
class CScheduler;
namespace interfaces {
//! Interface for giving wallet processes access to blockchain state.
@ -24,6 +26,24 @@ class ChainClient
{
public:
virtual ~ChainClient() {}
//! Register rpcs.
virtual void registerRpcs() = 0;
//! Check for errors before loading.
virtual bool verify() = 0;
//! Load saved state.
virtual bool load() = 0;
//! Start client execution and provide a scheduler.
virtual void start(CScheduler& scheduler) = 0;
//! Save state to disk.
virtual void flush() = 0;
//! Shut down client.
virtual void stop() = 0;
};
//! Return implementation of Chain interface.

View file

@ -7,6 +7,7 @@
#include <amount.h>
#include <chain.h>
#include <consensus/validation.h>
#include <init.h>
#include <interfaces/chain.h>
#include <interfaces/handler.h>
#include <net.h>
@ -14,6 +15,8 @@
#include <policy/fees.h>
#include <policy/policy.h>
#include <primitives/transaction.h>
#include <rpc/server.h>
#include <scheduler.h>
#include <script/ismine.h>
#include <script/standard.h>
#include <support/allocators/secure.h>
@ -25,7 +28,9 @@
#include <validation.h>
#include <wallet/feebumper.h>
#include <wallet/fees.h>
#include <wallet/rpcwallet.h>
#include <wallet/wallet.h>
#include <wallet/walletutil.h>
#include <memory>
#include <string>
@ -470,6 +475,13 @@ public:
: m_chain(chain), m_wallet_filenames(std::move(wallet_filenames))
{
}
void registerRpcs() override { return RegisterWalletRPCCommands(::tableRPC); }
bool verify() override { return VerifyWallets(m_chain, m_wallet_filenames); }
bool load() override { return LoadWallets(m_chain, m_wallet_filenames); }
void start(CScheduler& scheduler) override { return StartWallets(scheduler); }
void flush() override { return FlushWallets(); }
void stop() override { return StopWallets(); }
~WalletClientImpl() override { UnloadWallets(); }
Chain& m_chain;
std::vector<std::string> m_wallet_filenames;

View file

@ -5,6 +5,7 @@
#include <chainparams.h>
#include <init.h>
#include <interfaces/chain.h>
#include <net.h>
#include <scheduler.h>
#include <outputtype.h>
@ -28,28 +29,8 @@ public:
//! Wallets parameter interaction
bool ParameterInteraction() const override;
//! Register wallet RPCs.
void RegisterRPC(CRPCTable &tableRPC) const override;
//! Responsible for reading and validating the -wallet arguments and verifying the wallet database.
// This function will perform salvage on the wallet if requested, as long as only one wallet is
// being loaded (WalletParameterInteraction forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet).
bool Verify(interfaces::Chain& chain) const override;
//! Load wallet databases.
bool Open(interfaces::Chain& chain) const override;
//! Complete startup of wallets.
void Start(CScheduler& scheduler) const override;
//! Flush all wallets in preparation for shutdown.
void Flush() const override;
//! Stop all wallets. Wallets will be flushed first.
void Stop() const override;
//! Close all wallets.
void Close() const override;
//! Add wallets that should be opened to list of init interfaces.
void Construct(InitInterfaces& interfaces) const override;
};
const WalletInitInterface& g_wallet_init_interface = WalletInit();
@ -99,7 +80,6 @@ bool WalletInit::ParameterInteraction() const
return true;
}
gArgs.SoftSetArg("-wallet", "");
const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1;
if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) {
@ -165,21 +145,8 @@ bool WalletInit::ParameterInteraction() const
return true;
}
void WalletInit::RegisterRPC(CRPCTable &t) const
bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files)
{
if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) {
return;
}
RegisterWalletRPCCommands(t);
}
bool WalletInit::Verify(interfaces::Chain& chain) const
{
if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) {
return true;
}
if (gArgs.IsArgSet("-walletdir")) {
fs::path wallet_dir = gArgs.GetArg("-walletdir", "");
boost::system::error_code error;
@ -200,8 +167,6 @@ bool WalletInit::Verify(interfaces::Chain& chain) const
uiInterface.InitMessage(_("Verifying wallet(s)..."));
std::vector<std::string> wallet_files = gArgs.GetArgs("-wallet");
// Parameter interaction code should have thrown an error if -salvagewallet
// was enabled with more than wallet file, so the wallet_files size check
// here should have no effect.
@ -228,14 +193,19 @@ bool WalletInit::Verify(interfaces::Chain& chain) const
return true;
}
bool WalletInit::Open(interfaces::Chain& chain) const
void WalletInit::Construct(InitInterfaces& interfaces) const
{
if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) {
LogPrintf("Wallet disabled!\n");
return true;
return;
}
gArgs.SoftSetArg("-wallet", "");
interfaces.chain_clients.emplace_back(interfaces::MakeWalletClient(*interfaces.chain, gArgs.GetArgs("-wallet")));
}
for (const std::string& walletFile : gArgs.GetArgs("-wallet")) {
bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files)
{
for (const std::string& walletFile : wallet_files) {
std::shared_ptr<CWallet> pwallet = CWallet::CreateWalletFromFile(chain, WalletLocation(walletFile));
if (!pwallet) {
return false;
@ -246,7 +216,7 @@ bool WalletInit::Open(interfaces::Chain& chain) const
return true;
}
void WalletInit::Start(CScheduler& scheduler) const
void StartWallets(CScheduler& scheduler)
{
for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
pwallet->postInitProcess();
@ -256,21 +226,21 @@ void WalletInit::Start(CScheduler& scheduler) const
scheduler.scheduleEvery(MaybeCompactWalletDB, 500);
}
void WalletInit::Flush() const
void FlushWallets()
{
for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
pwallet->Flush(false);
}
}
void WalletInit::Stop() const
void StopWallets()
{
for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
pwallet->Flush(true);
}
}
void WalletInit::Close() const
void UnloadWallets()
{
for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
RemoveWallet(pwallet);

View file

@ -8,6 +8,8 @@
InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName): BasicTestingSetup(chainName)
{
m_chain_client = MakeWalletClient(*m_chain, {});
std::string sep;
sep += fs::path::preferred_separator;

View file

@ -18,6 +18,7 @@ struct InitWalletDirTestingSetup: public BasicTestingSetup {
fs::path m_cwd;
std::map<std::string, fs::path> m_walletdir_path_cases;
std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain();
std::unique_ptr<interfaces::ChainClient> m_chain_client;
};
#endif // BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H

View file

@ -17,7 +17,7 @@ BOOST_FIXTURE_TEST_SUITE(init_tests, InitWalletDirTestingSetup)
BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default)
{
SetWalletDir(m_walletdir_path_cases["default"]);
bool result = g_wallet_init_interface.Verify(*m_chain);
bool result = m_chain_client->verify();
BOOST_CHECK(result == true);
fs::path walletdir = gArgs.GetArg("-walletdir", "");
fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
@ -27,7 +27,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default)
BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom)
{
SetWalletDir(m_walletdir_path_cases["custom"]);
bool result = g_wallet_init_interface.Verify(*m_chain);
bool result = m_chain_client->verify();
BOOST_CHECK(result == true);
fs::path walletdir = gArgs.GetArg("-walletdir", "");
fs::path expected_path = fs::canonical(m_walletdir_path_cases["custom"]);
@ -37,28 +37,28 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom)
BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_does_not_exist)
{
SetWalletDir(m_walletdir_path_cases["nonexistent"]);
bool result = g_wallet_init_interface.Verify(*m_chain);
bool result = m_chain_client->verify();
BOOST_CHECK(result == false);
}
BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_directory)
{
SetWalletDir(m_walletdir_path_cases["file"]);
bool result = g_wallet_init_interface.Verify(*m_chain);
bool result = m_chain_client->verify();
BOOST_CHECK(result == false);
}
BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative)
{
SetWalletDir(m_walletdir_path_cases["relative"]);
bool result = g_wallet_init_interface.Verify(*m_chain);
bool result = m_chain_client->verify();
BOOST_CHECK(result == false);
}
BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing)
{
SetWalletDir(m_walletdir_path_cases["trailing"]);
bool result = g_wallet_init_interface.Verify(*m_chain);
bool result = m_chain_client->verify();
BOOST_CHECK(result == true);
fs::path walletdir = gArgs.GetArg("-walletdir", "");
fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing)
BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2)
{
SetWalletDir(m_walletdir_path_cases["trailing2"]);
bool result = g_wallet_init_interface.Verify(*m_chain);
bool result = m_chain_client->verify();
BOOST_CHECK(result == true);
fs::path walletdir = gArgs.GetArg("-walletdir", "");
fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);

View file

@ -37,6 +37,27 @@ namespace interfaces {
class Chain;
} // namespace interfaces
//! Responsible for reading and validating the -wallet arguments and verifying the wallet database.
// This function will perform salvage on the wallet if requested, as long as only one wallet is
// being loaded (WalletParameterInteraction forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet).
bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files);
//! Load wallet databases.
bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files);
//! Complete startup of wallets.
void StartWallets(CScheduler& scheduler);
//! Flush all wallets in preparation for shutdown.
void FlushWallets();
//! Stop all wallets. Wallets will be flushed first.
void StopWallets();
//! Close all wallets.
void UnloadWallets();
bool AddWallet(const std::shared_ptr<CWallet>& wallet);
bool RemoveWallet(const std::shared_ptr<CWallet>& wallet);
bool HasWallets();

View file

@ -9,10 +9,7 @@
class CScheduler;
class CRPCTable;
namespace interfaces {
class Chain;
} // namespace interfaces
struct InitInterfaces;
class WalletInitInterface {
public:
@ -22,20 +19,8 @@ public:
virtual void AddWalletOptions() const = 0;
/** Check wallet parameter interaction */
virtual bool ParameterInteraction() const = 0;
/** Register wallet RPC*/
virtual void RegisterRPC(CRPCTable &) const = 0;
/** Verify wallets */
virtual bool Verify(interfaces::Chain& chain) const = 0;
/** Open wallets*/
virtual bool Open(interfaces::Chain& chain) const = 0;
/** Start wallets*/
virtual void Start(CScheduler& scheduler) const = 0;
/** Flush Wallets*/
virtual void Flush() const = 0;
/** Stop Wallets*/
virtual void Stop() const = 0;
/** Close wallets */
virtual void Close() const = 0;
/** Add wallets that should be opened to list of init interfaces. */
virtual void Construct(InitInterfaces& interfaces) const = 0;
virtual ~WalletInitInterface() {}
};