Apologies for the mostly code move only PR. This is a pre-req for both #10740 and #10762
All wallet component initialization/destruction functions are now in their own wallet/init.cpp translation unit and are no longer static functions on the CWallet class. The bitcoin_server also no longer has any knowledge that there are multiple wallets in vpwallet.
There should be no changes in behavior from this PR.
186@@ -187,11 +187,6 @@ void Shutdown()
187 StopREST();
188 StopRPC();
189 StopHTTPServer();
190-#ifdef ENABLE_WALLET
Correct - I don’t believe the early wallet flush is necessary (although I may be wrong). See PR description:
The most important one is that Flush() no longer gets called twice on shutdown. I think this is fine, and all the tests pass, but this probably requires some careful consideration from people who are more familiar with the wallet code.
224@@ -230,9 +225,7 @@ void Shutdown()
225 pblocktree = NULL;
226 }
227 #ifdef ENABLE_WALLET
#ifdef ENABLE_WALLET to walletinit.cpp?
Rebased on @ryanofsky’s #10976 and reduced scope. This PR now only moves the remaining wallet startup/shutdown functions to wallet/init.cpp in preparation for #10740 (dynamic loading/unloading of wallets) and #10762 (remove wallet dependencies from init.cpp).
There should be no change in behavior from this PR.
21@@ -22,4 +22,7 @@ bool WalletVerify();
22 //! Load wallet databases.
23 bool InitLoadWallet();
24
25+//! Flush all wallets in preparation for shutdown.
26+// Call with shutdown = true to actually shutdown the wallet.
In commit “[wallet] move wallet flush calls to wallet/init.cpp”
I think you need to use //! prefix on each line of the comment for doxygen to pick it up.
264@@ -265,10 +265,7 @@ void Shutdown()
265 UnregisterAllValidationInterfaces();
266 GetMainSignals().UnregisterBackgroundSignalScheduler();
267 #ifdef ENABLE_WALLET
268- for (CWalletRef pwallet : vpwallets) {
269- delete pwallet;
270- }
271- vpwallets.clear();
272+ DetachWallets();
In commit “[wallet] move wallet detach calls to wallet/init.cpp”
Maybe go with “close” or “free” instead of “detach”. Detach sounds like it could be keeping the wallets open but handing them off.
FreeWallets.
5@@ -6,14 +6,22 @@
6 #ifndef BITCOIN_WALLET_INIT_H
7 #define BITCOIN_WALLET_INIT_H
8
9+#include "wallet/rpcwallet.h"
252@@ -245,3 +253,28 @@ bool InitLoadWallet()
253
254 return true;
255 }
256+
257+void WalletCompleteStartup(CScheduler& scheduler) {
258+ for (CWalletRef pwallet : vpwallets) {
CWalletRef.
38+
39+//! Shutdown all wallets. Wallets will be flushed first.
40+void ShutdownWallets();
41+
42+//! Detach all wallets
43+void DetachWallets();
37+void FlushWallets();
38+
39+//! Shutdown all wallets. Wallets will be flushed first.
40+void ShutdownWallets();
41+
42+//! Detach all wallets
29@@ -22,4 +30,15 @@ bool WalletVerify();
30 //! Load wallet databases.
31 bool InitLoadWallet();
32
33+//! Complete startup of wallets
17 std::string GetWalletHelpString(bool showDebug);
18
19 //! Wallets parameter interaction
20 bool WalletParameterInteraction();
21
22+//! Register wallet RPCs
Re: naming of unload/close/free/detach wallet function. I want the wallet loading/unloading RPC verbs to be antonyms so it’s obvious they’re carrying out opposite actions. I started with load/unload, but dropped that to avoid any ambiguity with adding keys and addresses to an already open wallet. attach/detach is bad for the reason Russ mentioned. Is everyone happy with open/close?
(I guess the name of the function doesn’t need to map exactly to the name of the RPC, but I think it makes sense to do that if we can)
utACK ca94bf2c39b06f1ed643c24dfcd78ef5af00562a. Note that what you’re calling flush, I’m calling stop in #10973, and what you’re calling stop, I’m calling shutdown. But I’m not too worried about the names.
#10973 links: declarations, definitions
Thanks @ryanofsky .
Other reviewers/maintainers - I’m happy to rebase/squash as Russ suggests if that makes things easier to review. Please let me know if you want that.
43@@ -44,7 +44,6 @@
44 #include "validationinterface.h"
45 #ifdef ENABLE_WALLET
46 #include "wallet/init.h"
47-#include "wallet/wallet.h"
1558@@ -1567,7 +1559,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
1559
1560 // ********************************************************* Step 8: load wallet
1561 #ifdef ENABLE_WALLET
1562- if (!InitLoadWallet())
1563+ if (!OpenWallets())
20+//! Register wallet RPCs.
21+void RegisterWalletRPC(CRPCTable &tableRPC);
22+
23 //! Responsible for reading and validating the -wallet arguments and verifying the wallet database.
24 // This function will perform salvage on the wallet if requested, as long as only one wallet is
25 // being loaded (CWallet::ParameterInteraction forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet).
Rationale:
- this init function can now open multiple wallets (hence
Wallet->Wallets)
- This is named as the antonym to CloseWallets(), which carries out the
opposite action.
This function can now verify multiple wallets.