diff options
author | Samuel Dobson <dobsonsa68@gmail.com> | 2020-07-11 23:11:15 +1200 |
---|---|---|
committer | Samuel Dobson <dobsonsa68@gmail.com> | 2020-07-11 23:23:28 +1200 |
commit | 5f96bce9b7f38c687817d58e8b54a5b7ebfe91b3 (patch) | |
tree | 9edeacae57242b6dff3028adc8b867e9d88adf1a /src | |
parent | 89899a344861c27ad31dc09334fa88ec9402dba4 (diff) | |
parent | fa73493930e35850e877725167dc9d42e47015c8 (diff) | |
download | bitcoin-5f96bce9b7f38c687817d58e8b54a5b7ebfe91b3.tar.xz |
Merge #18923: wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off
fa73493930e35850e877725167dc9d42e47015c8 refactor: Use C++11 range-based for loop (MarcoFalke)
fa7b164d62d9f12e9cda79bf28bf435acf2d1e38 wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off (MarcoFalke)
faf8401c195f52470d1ca6e2c94cb3820e57ee41 wallet: Pass unused args to StartWallets (MarcoFalke)
fa6c186436337c8ed7d9e1ab065377f8cda5c0b7 gui tests: Limit life-time of dummy testing setup (MarcoFalke)
fa28a618972911239a119248ab1194702a5c36d8 test: Add smoke test to check that wallets are flushed by default (MarcoFalke)
Pull request description:
User-facing, this is a refactor. Internally, the scheduler does not have to call a mostly empty function every half a second.
ACKs for top commit:
jnewbery:
utACK fa73493930e35850e877725167dc9d42e47015c8
meshcollider:
utACK fa73493930e35850e877725167dc9d42e47015c8
ryanofsky:
Code review ACK fa73493930e35850e877725167dc9d42e47015c8. Just rebased since last review
Tree-SHA512: 99e1fe1b2c22a3f4b19de3e566241d38693f4fd8d5a68ba1838d86740aa6c08e3325c11a072e30fd262a8861af4278bed52eb9374c85179b8f536477f528247c
Diffstat (limited to 'src')
-rw-r--r-- | src/interfaces/chain.h | 3 | ||||
-rw-r--r-- | src/interfaces/wallet.cpp | 11 | ||||
-rw-r--r-- | src/qt/test/apptests.cpp | 8 | ||||
-rw-r--r-- | src/qt/test/test_main.cpp | 4 | ||||
-rw-r--r-- | src/wallet/bdb.cpp | 4 | ||||
-rw-r--r-- | src/wallet/context.h | 2 | ||||
-rw-r--r-- | src/wallet/init.cpp | 12 | ||||
-rw-r--r-- | src/wallet/load.cpp | 7 | ||||
-rw-r--r-- | src/wallet/load.h | 3 | ||||
-rw-r--r-- | src/wallet/test/init_test_fixture.cpp | 5 | ||||
-rw-r--r-- | src/wallet/test/wallet_test_fixture.h | 5 | ||||
-rw-r--r-- | src/wallet/walletdb.cpp | 3 |
12 files changed, 40 insertions, 27 deletions
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 65695707f7..bbeb0fa801 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -15,6 +15,7 @@ #include <string> #include <vector> +class ArgsManager; class CBlock; class CFeeRate; class CRPCCommand; @@ -322,7 +323,7 @@ std::unique_ptr<Chain> MakeChain(NodeContext& node); //! analysis, or fee estimation. These clients need to expose their own //! MakeXXXClient functions returning their implementations of the ChainClient //! interface. -std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, std::vector<std::string> wallet_filenames); +std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames); } // namespace interfaces diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index f6806aed65..7fd24425cf 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -483,10 +483,11 @@ public: class WalletClientImpl : public ChainClient { public: - WalletClientImpl(Chain& chain, std::vector<std::string> wallet_filenames) + WalletClientImpl(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) : m_wallet_filenames(std::move(wallet_filenames)) { m_context.chain = &chain; + m_context.args = &args; } void registerRpcs() override { @@ -499,7 +500,7 @@ public: } bool verify() override { return VerifyWallets(*m_context.chain, m_wallet_filenames); } bool load() override { return LoadWallets(*m_context.chain, m_wallet_filenames); } - void start(CScheduler& scheduler) override { return StartWallets(scheduler); } + void start(CScheduler& scheduler) override { return StartWallets(scheduler, *Assert(m_context.args)); } void flush() override { return FlushWallets(); } void stop() override { return StopWallets(); } void setMockTime(int64_t time) override { return SetMockTime(time); } @@ -514,7 +515,7 @@ public: ~WalletClientImpl() override { UnloadWallets(); } WalletContext m_context; - std::vector<std::string> m_wallet_filenames; + const std::vector<std::string> m_wallet_filenames; std::vector<std::unique_ptr<Handler>> m_rpc_handlers; std::list<CRPCCommand> m_rpc_commands; }; @@ -523,9 +524,9 @@ public: std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<WalletImpl>(wallet) : nullptr; } -std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, std::vector<std::string> wallet_filenames) +std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) { - return MakeUnique<WalletClientImpl>(chain, std::move(wallet_filenames)); + return MakeUnique<WalletClientImpl>(chain, args, std::move(wallet_filenames)); } } // namespace interfaces diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index f88d57c716..443e2d047d 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -62,9 +62,10 @@ void AppTests::appTests() } #endif - BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to - ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference - LogInstance().DisconnectTestLogger(); + fs::create_directories([] { + BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to + return GetDataDir() / "blocks"; + }()); m_app.parameterSetup(); m_app.createOptionsModel(true /* reset settings */); @@ -80,6 +81,7 @@ void AppTests::appTests() m_app.exec(); // Reset global state to avoid interfering with later tests. + LogInstance().DisconnectTestLogger(); AbortShutdown(); UnloadBlockIndex(); WITH_LOCK(::cs_main, g_chainman.Reset()); diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index aefdcd2716..12efca2503 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -40,7 +40,7 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin); const std::function<void(const std::string&)> G_TEST_LOG_FUN{}; // This is all you need to run all the tests -int main(int argc, char *argv[]) +int main(int argc, char* argv[]) { // Initialize persistent globals with the testing setup state for sanity. // E.g. -datadir in gArgs is set to a temp directory dummy value (instead @@ -70,6 +70,8 @@ int main(int argc, char *argv[]) BitcoinApplication app(*node); app.setApplicationName("Bitcoin-Qt-test"); + node->setupServerArgs(); // Make gArgs available in the NodeContext + node->context()->args->ClearArgs(); // Clear added args again AppTests app_tests(app); if (QTest::qExec(&app_tests) != 0) { fInvalid = true; diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 5f823d5906..54a9aa34e0 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -623,8 +623,8 @@ bool BerkeleyDatabase::PeriodicFlush() if (!lockDb) return false; // Don't flush if any databases are in use - for (auto it = env->mapFileUseCount.begin() ; it != env->mapFileUseCount.end(); it++) { - if ((*it).second > 0) return false; + for (const auto& use_count : env->mapFileUseCount) { + if (use_count.second > 0) return false; } // Don't flush if there haven't been any batch writes for this database. diff --git a/src/wallet/context.h b/src/wallet/context.h index 3c8fdd1c59..a83591154f 100644 --- a/src/wallet/context.h +++ b/src/wallet/context.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_WALLET_CONTEXT_H #define BITCOIN_WALLET_CONTEXT_H +class ArgsManager; namespace interfaces { class Chain; } // namespace interfaces @@ -21,6 +22,7 @@ class Chain; //! behavior. struct WalletContext { interfaces::Chain* chain{nullptr}; + ArgsManager* args{nullptr}; //! Declare default constructor and destructor that are not inline, so code //! instantiating the WalletContext struct doesn't need to #include class diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index f173b5e62b..781920755c 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -9,6 +9,7 @@ #include <node/context.h> #include <node/ui_interface.h> #include <outputtype.h> +#include <util/check.h> #include <util/moneystr.h> #include <util/system.h> #include <util/translation.h> @@ -16,9 +17,9 @@ #include <wallet/wallet.h> #include <walletinitinterface.h> -class WalletInit : public WalletInitInterface { +class WalletInit : public WalletInitInterface +{ public: - //! Was the wallet component compiled in. bool HasWalletSupport() const override {return true;} @@ -112,10 +113,11 @@ bool WalletInit::ParameterInteraction() const void WalletInit::Construct(NodeContext& node) const { - if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { + ArgsManager& args = *Assert(node.args); + if (args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { LogPrintf("Wallet disabled!\n"); return; } - gArgs.SoftSetArg("-wallet", ""); - node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, gArgs.GetArgs("-wallet"))); + args.SoftSetArg("-wallet", ""); + node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"))); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 8df3e78215..c2818a41e7 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -11,6 +11,7 @@ #include <util/system.h> #include <util/translation.h> #include <wallet/wallet.h> +#include <wallet/walletdb.h> bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files) { @@ -82,14 +83,16 @@ bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& walle } } -void StartWallets(CScheduler& scheduler) +void StartWallets(CScheduler& scheduler, const ArgsManager& args) { for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { pwallet->postInitProcess(); } // Schedule periodic wallet flushes and tx rebroadcasts - scheduler.scheduleEvery(MaybeCompactWalletDB, std::chrono::milliseconds{500}); + if (args.GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { + scheduler.scheduleEvery(MaybeCompactWalletDB, std::chrono::milliseconds{500}); + } scheduler.scheduleEvery(MaybeResendWalletTxs, std::chrono::milliseconds{1000}); } diff --git a/src/wallet/load.h b/src/wallet/load.h index e24b1f2e69..ff4f5b4b23 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -9,6 +9,7 @@ #include <string> #include <vector> +class ArgsManager; class CScheduler; namespace interfaces { @@ -22,7 +23,7 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files); //! Complete startup of wallets. -void StartWallets(CScheduler& scheduler); +void StartWallets(CScheduler& scheduler, const ArgsManager& args); //! Flush all wallets in preparation for shutdown. void FlushWallets(); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 797a0d634f..35bd965673 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -3,13 +3,14 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <fs.h> +#include <util/check.h> #include <util/system.h> #include <wallet/test/init_test_fixture.h> -InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName): BasicTestingSetup(chainName) +InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - m_chain_client = MakeWalletClient(*m_chain, {}); + m_chain_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {}); std::string sep; sep += fs::path::preferred_separator; diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index 6c32868b1e..99d7cfe921 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -10,17 +10,18 @@ #include <interfaces/chain.h> #include <interfaces/wallet.h> #include <node/context.h> +#include <util/check.h> #include <wallet/wallet.h> #include <memory> /** Testing setup and teardown for wallet. */ -struct WalletTestingSetup: public TestingSetup { +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::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, {}); + std::unique_ptr<interfaces::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {}); CWallet m_wallet; std::unique_ptr<interfaces::Handler> m_chain_notifications_handler; }; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 5297c43718..9d53f0df9c 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -952,9 +952,6 @@ void MaybeCompactWalletDB() if (fOneThread.exchange(true)) { return; } - if (!gArgs.GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { - return; - } for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { WalletDatabase& dbh = pwallet->GetDBHandle(); |