Fix wallet directory initialization

util/system datadir code should not care about wallets or create any wallet paths
If -walletdir is specified, wallet init code should use that directory
If -walletdir is specified and path is not a directory, return a fatal error
If -walletdir is not specified, use <datadir>/wallets if it is directory.
If -walletdir is not specified and <datadir>/wallets does not exist:
<datadir>/wallets wallet should call ListWalletDir ListDatabases on <datadir>
If list is empty wallet should create <datadir>/wallets as a directory,
otherwise it should create it as a symlink pointing to .
discussion at bitcoin#16220

src/common/init.cpp

@@ -45,21 +45,11 @@ std::optional<ConfigError> InitConfig(ArgsManager& args, SettingsAbortFn setting
         // Create datadir if it does not exist.
         const auto base_path{args.GetDataDirBase()};
         if (!fs::exists(base_path)) {
-            // When creating a *new* datadir, also create a "wallets" subdirectory,
-            // whether or not the wallet is enabled now, so if the wallet is enabled
-            // in the future, it will use the "wallets" subdirectory for creating
-            // and listing wallets, rather than the top-level directory where
-            // wallets could be mixed up with other files. For backwards
-            // compatibility, wallet code will use the "wallets" subdirectory only
-            // if it already exists, but never create it itself. There is discussion
-            // in https://github.com/bitcoin/bitcoin/issues/16220 about ways to
-            // change wallet code so it would no longer be necessary to create
-            // "wallets" subdirectories here.
-            fs::create_directories(base_path / "wallets");
+            fs::create_directories(base_path);
         }
         const auto net_path{args.GetDataDirNet()};
         if (!fs::exists(net_path)) {
-            fs::create_directories(net_path / "wallets");
+            fs::create_directories(net_path);
         }
 
         // Show an error or warning if there is a bitcoin.conf file in the

src/dummywallet.cpp

@@ -5,6 +5,7 @@
 #include <common/args.h>
 #include <logging.h>
 #include <walletinitinterface.h>
+#include <util/translation.h>
 
 class ArgsManager;
 
@@ -22,6 +23,7 @@ class DummyWalletInit : public WalletInitInterface {
     void AddWalletOptions(ArgsManager& argsman) const override;
     bool ParameterInteraction() const override {return true;}
     void Construct(node::NodeContext& node) const override {LogPrintf("No wallet support compiled in!\n");}
+    void InitWalletDir(node::NodeContext& node, bilingual_str& error) const override {LogPrintf("No wallet support compiled in!\n");}
 };
 
 void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const

src/init.cpp

@@ -1141,6 +1141,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     g_wallet_init_interface.Construct(node);
     uiInterface.InitWallet();
 
+    // Initialize wallet directory
+    bilingual_str error;
+    g_wallet_init_interface.InitWalletDir(node, error);
+    if (!error.empty()) {
+        return InitError(error);
+    }
+
     /* Register RPC commands regardless of -server setting so they will be
      * available in the GUI RPC console even if external calls are disabled.
      */

src/wallet/init.cpp

@@ -40,6 +40,9 @@ class WalletInit : public WalletInitInterface
 
     //! Add wallets that should be opened to list of chain clients.
     void Construct(NodeContext& node) const override;
+
+    //! Initialize wallet directory
+    void InitWalletDir(NodeContext& node, bilingual_str& error) const override;
 };
 
 void WalletInit::AddWalletOptions(ArgsManager& argsman) const
@@ -136,6 +139,23 @@ void WalletInit::Construct(NodeContext& node) const
     node.wallet_loader = wallet_loader.get();
     node.chain_clients.emplace_back(std::move(wallet_loader));
 }
+
+void WalletInit::InitWalletDir(NodeContext& node, bilingual_str& error) const
+{
+    ArgsManager& args = *Assert(node.args);
+
+    fs::path path;
+
+    if (!args.IsArgSet("-walletdir")) {
+        path = args.GetDataDirNet();
+        if (!fs::is_directory(path / "wallets")) {
+            std::vector<fs::path> db_paths = ListDatabases(path);
+            if (db_paths.empty()) {
+                fs::create_directories(path / "wallets");
+            }
+        }
+    }
+}
 } // namespace wallet
 
 const WalletInitInterface& g_wallet_init_interface = wallet::WalletInit();

src/walletinitinterface.h

@@ -21,6 +21,8 @@ class WalletInitInterface {
     virtual bool ParameterInteraction() const = 0;
     /** Add wallets that should be opened to list of chain clients. */
     virtual void Construct(node::NodeContext& node) const = 0;
+    /** Initialize wallet directory */
+    virtual void InitWalletDir(node::NodeContext& node, bilingual_str& error) const = 0;
 
     virtual ~WalletInitInterface() {}
 };

test/functional/wallet_listtransactions.py

@@ -234,8 +234,8 @@ def run_externally_generated_address_test(self):
         # refill keypool otherwise the second node wouldn't recognize addresses generated on the first nodes
         self.nodes[0].keypoolrefill(1000)
         self.stop_nodes()
-        wallet0 = os.path.join(self.nodes[0].chain_path, self.default_wallet_name, "wallet.dat")
-        wallet2 = os.path.join(self.nodes[2].chain_path, self.default_wallet_name, "wallet.dat")
+        wallet0 = os.path.join(self.nodes[0].chain_path, "wallets", self.default_wallet_name, "wallet.dat")
+        wallet2 = os.path.join(self.nodes[2].chain_path, "wallets", self.default_wallet_name, "wallet.dat")
         shutil.copyfile(wallet0, wallet2)
         self.start_nodes()
         # reconnect nodes