Merge #19137: wallettool: Add dump and createfromdump commands

23cac24dd3 tests: Test bitcoin-wallet dump and createfromdump (Andrew Chow)
a88c320041 wallettool: Add createfromdump command (Andrew Chow)
e1e7a90d5f wallettool: Add dump command (Andrew Chow)

Pull request description:

  Adds two commands to the `bitcoin-wallet` tool: `dump` and `createfromdump`. These commands will be useful for a wallet storage migration in the future. It is also generally useful to have a storage agnostic dump like this. These commands are similar to BDB's `db_dump` and `db_load` tools. This can also be useful for manual construction of a wallet file for tests.

  `dump` outputs every key-value pair from the wallet as comma separated hex. Each key-value pair is on its own line with the key and value in hex separated by a comma. This is output to the file specified by the new `-dumpfile` option.

  `createfromdump` takes a file produced by `dump` and creates a new wallet file with exactly the records specified in that file.

  A new option `-dumpfile` is added to the wallet tool. When used with `dump`, the records will be written to the specified file. When used with `createfromdump`, the file is read and the key-value pairs constructed from it. `createfromdump` requires `-dumpfile`.

  A simple round-trip test is added to the `tool_wallet.py`.

  This PR is based on #19334,

ACKs for top commit:
  Sjors:
    re-utACK 23cac24
  MarcoFalke:
    re review ACK 23cac24dd3 only change is rebase and removing useless shared_ptr wrapper 🎼
  ryanofsky:
    Code review ACK 23cac24dd3. Only changes since last review rebase and changing a pointer to a reference

Tree-SHA512: 2d63cf62baca3d16495aa698dc02f7d889c81b41015e9c92c23c275bb4a690fc176d351c3fd7f310bd6b17f5a936cc9be694cbecd702af741b96c0f530e72fa2

src/Makefile.am

@@ -249,6 +249,7 @@ BITCOIN_CORE_H = \
   wallet/context.h \
   wallet/crypter.h \
   wallet/db.h \
+  wallet/dump.h \
   wallet/feebumper.h \
   wallet/fees.h \
   wallet/ismine.h \
@@ -361,6 +362,7 @@ libbitcoin_wallet_a_SOURCES = \
   wallet/context.cpp \
   wallet/crypter.cpp \
   wallet/db.cpp \
+  wallet/dump.cpp \
   wallet/feebumper.cpp \
   wallet/fees.cpp \
   wallet/interfaces.cpp \

src/bitcoin-wallet.cpp

@@ -27,13 +27,17 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
     argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-wallet=<wallet-name>", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-dumpfile=<file name>", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_STRING, OptionsCategory::OPTIONS);
     argsman.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-descriptors", "Create descriptors wallet. Only for create", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
+    argsman.AddArg("-format=<format>", "The format of the wallet file to create. Either \"bdb\" or \"sqlite\". Only used with 'createfromdump'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
 
     argsman.AddArg("info", "Get wallet info", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
     argsman.AddArg("create", "Create new wallet file", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
     argsman.AddArg("salvage", "Attempt to recover private keys from a corrupt wallet. Warning: 'salvage' is experimental.", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
+    argsman.AddArg("dump", "Print out all of the wallet key-value records", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
+    argsman.AddArg("createfromdump", "Create new wallet file from dumped records", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS);
 }
 
 static bool WalletAppInit(int argc, char* argv[])

src/wallet/dump.cpp

@@ -0,0 +1,282 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <wallet/dump.h>
+
+#include <util/translation.h>
+#include <wallet/wallet.h>
+
+static const std::string DUMP_MAGIC = "BITCOIN_CORE_WALLET_DUMP";
+uint32_t DUMP_VERSION = 1;
+
+bool DumpWallet(CWallet& wallet, bilingual_str& error)
+{
+    // Get the dumpfile
+    std::string dump_filename = gArgs.GetArg("-dumpfile", "");
+    if (dump_filename.empty()) {
+        error = _("No dump file provided. To use dump, -dumpfile=<filename> must be provided.");
+        return false;
+    }
+
+    fs::path path = dump_filename;
+    path = fs::absolute(path);
+    if (fs::exists(path)) {
+        error = strprintf(_("File %s already exists. If you are sure this is what you want, move it out of the way first."), path.string());
+        return false;
+    }
+    fsbridge::ofstream dump_file;
+    dump_file.open(path);
+    if (dump_file.fail()) {
+        error = strprintf(_("Unable to open %s for writing"), path.string());
+        return false;
+    }
+
+    CHashWriter hasher(0, 0);
+
+    WalletDatabase& db = wallet.GetDatabase();
+    std::unique_ptr<DatabaseBatch> batch = db.MakeBatch();
+
+    bool ret = true;
+    if (!batch->StartCursor()) {
+        error = _("Error: Couldn't create cursor into database");
+        ret = false;
+    }
+
+    // Write out a magic string with version
+    std::string line = strprintf("%s,%u\n", DUMP_MAGIC, DUMP_VERSION);
+    dump_file.write(line.data(), line.size());
+    hasher.write(line.data(), line.size());
+
+    // Write out the file format
+    line = strprintf("%s,%s\n", "format", db.Format());
+    dump_file.write(line.data(), line.size());
+    hasher.write(line.data(), line.size());
+
+    if (ret) {
+
+        // Read the records
+        while (true) {
+            CDataStream ss_key(SER_DISK, CLIENT_VERSION);
+            CDataStream ss_value(SER_DISK, CLIENT_VERSION);
+            bool complete;
+            ret = batch->ReadAtCursor(ss_key, ss_value, complete);
+            if (complete) {
+                ret = true;
+                break;
+            } else if (!ret) {
+                error = _("Error reading next record from wallet database");
+                break;
+            }
+            std::string key_str = HexStr(ss_key);
+            std::string value_str = HexStr(ss_value);
+            line = strprintf("%s,%s\n", key_str, value_str);
+            dump_file.write(line.data(), line.size());
+            hasher.write(line.data(), line.size());
+        }
+    }
+
+    batch->CloseCursor();
+    batch.reset();
+
+    // Close the wallet after we're done with it. The caller won't be doing this
+    wallet.Close();
+
+    if (ret) {
+        // Write the hash
+        tfm::format(dump_file, "checksum,%s\n", HexStr(hasher.GetHash()));
+        dump_file.close();
+    } else {
+        // Remove the dumpfile on failure
+        dump_file.close();
+        fs::remove(path);
+    }
+
+    return ret;
+}
+
+// The standard wallet deleter function blocks on the validation interface
+// queue, which doesn't exist for the bitcoin-wallet. Define our own
+// deleter here.
+static void WalletToolReleaseWallet(CWallet* wallet)
+{
+    wallet->WalletLogPrintf("Releasing wallet\n");
+    wallet->Close();
+    delete wallet;
+}
+
+bool CreateFromDump(const std::string& name, const fs::path& wallet_path, bilingual_str& error, std::vector<bilingual_str>& warnings)
+{
+    // Get the dumpfile
+    std::string dump_filename = gArgs.GetArg("-dumpfile", "");
+    if (dump_filename.empty()) {
+        error = _("No dump file provided. To use createfromdump, -dumpfile=<filename> must be provided.");
+        return false;
+    }
+
+    fs::path dump_path = dump_filename;
+    dump_path = fs::absolute(dump_path);
+    if (!fs::exists(dump_path)) {
+        error = strprintf(_("Dump file %s does not exist."), dump_path.string());
+        return false;
+    }
+    fsbridge::ifstream dump_file(dump_path);
+
+    // Compute the checksum
+    CHashWriter hasher(0, 0);
+    uint256 checksum;
+
+    // Check the magic and version
+    std::string magic_key;
+    std::getline(dump_file, magic_key, ',');
+    std::string version_value;
+    std::getline(dump_file, version_value, '\n');
+    if (magic_key != DUMP_MAGIC) {
+        error = strprintf(_("Error: Dumpfile identifier record is incorrect. Got \"%s\", expected \"%s\"."), magic_key, DUMP_MAGIC);
+        dump_file.close();
+        return false;
+    }
+    // Check the version number (value of first record)
+    uint32_t ver;
+    if (!ParseUInt32(version_value, &ver)) {
+        error =strprintf(_("Error: Unable to parse version %u as a uint32_t"), version_value);
+        dump_file.close();
+        return false;
+    }
+    if (ver != DUMP_VERSION) {
+        error = strprintf(_("Error: Dumpfile version is not supported. This version of bitcoin-wallet only supports version 1 dumpfiles. Got dumpfile with version %s"), version_value);
+        dump_file.close();
+        return false;
+    }
+    std::string magic_hasher_line = strprintf("%s,%s\n", magic_key, version_value);
+    hasher.write(magic_hasher_line.data(), magic_hasher_line.size());
+
+    // Get the stored file format
+    std::string format_key;
+    std::getline(dump_file, format_key, ',');
+    std::string format_value;
+    std::getline(dump_file, format_value, '\n');
+    if (format_key != "format") {
+        error = strprintf(_("Error: Dumpfile format record is incorrect. Got \"%s\", expected \"format\"."), format_key);
+        dump_file.close();
+        return false;
+    }
+    // Get the data file format with format_value as the default
+    std::string file_format = gArgs.GetArg("-format", format_value);
+    if (file_format.empty()) {
+        error = _("No wallet file format provided. To use createfromdump, -format=<format> must be provided.");
+        return false;
+    }
+    DatabaseFormat data_format;
+    if (file_format == "bdb") {
+        data_format = DatabaseFormat::BERKELEY;
+    } else if (file_format == "sqlite") {
+        data_format = DatabaseFormat::SQLITE;
+    } else {
+        error = strprintf(_("Unknown wallet file format \"%s\" provided. Please provide one of \"bdb\" or \"sqlite\"."), file_format);
+        return false;
+    }
+    if (file_format != format_value) {
+        warnings.push_back(strprintf(_("Warning: Dumpfile wallet format \"%s\" does not match command line specified format \"%s\"."), format_value, file_format));
+    }
+    std::string format_hasher_line = strprintf("%s,%s\n", format_key, format_value);
+    hasher.write(format_hasher_line.data(), format_hasher_line.size());
+
+    DatabaseOptions options;
+    DatabaseStatus status;
+    options.require_create = true;
+    options.require_format = data_format;
+    std::unique_ptr<WalletDatabase> database = MakeDatabase(wallet_path, options, status, error);
+    if (!database) return false;
+
+    // dummy chain interface
+    bool ret = true;
+    std::shared_ptr<CWallet> wallet(new CWallet(nullptr /* chain */, name, std::move(database)), WalletToolReleaseWallet);
+    {
+        LOCK(wallet->cs_wallet);
+        bool first_run = true;
+        DBErrors load_wallet_ret = wallet->LoadWallet(first_run);
+        if (load_wallet_ret != DBErrors::LOAD_OK) {
+            error = strprintf(_("Error creating %s"), name);
+            return false;
+        }
+
+        // Get the database handle
+        WalletDatabase& db = wallet->GetDatabase();
+        std::unique_ptr<DatabaseBatch> batch = db.MakeBatch();
+        batch->TxnBegin();
+
+        // Read the records from the dump file and write them to the database
+        while (dump_file.good()) {
+            std::string key;
+            std::getline(dump_file, key, ',');
+            std::string value;
+            std::getline(dump_file, value, '\n');
+
+            if (key == "checksum") {
+                std::vector<unsigned char> parsed_checksum = ParseHex(value);
+                std::copy(parsed_checksum.begin(), parsed_checksum.end(), checksum.begin());
+                break;
+            }
+
+            std::string line = strprintf("%s,%s\n", key, value);
+            hasher.write(line.data(), line.size());
+
+            if (key.empty() || value.empty()) {
+                continue;
+            }
+
+            if (!IsHex(key)) {
+                error = strprintf(_("Error: Got key that was not hex: %s"), key);
+                ret = false;
+                break;
+            }
+            if (!IsHex(value)) {
+                error = strprintf(_("Error: Got value that was not hex: %s"), value);
+                ret = false;
+                break;
+            }
+
+            std::vector<unsigned char> k = ParseHex(key);
+            std::vector<unsigned char> v = ParseHex(value);
+
+            CDataStream ss_key(k, SER_DISK, CLIENT_VERSION);
+            CDataStream ss_value(v, SER_DISK, CLIENT_VERSION);
+
+            if (!batch->Write(ss_key, ss_value)) {
+                error = strprintf(_("Error: Unable to write record to new wallet"));
+                ret = false;
+                break;
+            }
+        }
+
+        if (ret) {
+            uint256 comp_checksum = hasher.GetHash();
+            if (checksum.IsNull()) {
+                error = _("Error: Missing checksum");
+                ret = false;
+            } else if (checksum != comp_checksum) {
+                error = strprintf(_("Error: Dumpfile checksum does not match. Computed %s, expected %s"), HexStr(comp_checksum), HexStr(checksum));
+                ret = false;
+            }
+        }
+
+        if (ret) {
+            batch->TxnCommit();
+        } else {
+            batch->TxnAbort();
+        }
+
+        batch.reset();
+
+        dump_file.close();
+    }
+    wallet.reset(); // The pointer deleter will close the wallet for us.
+
+    // Remove the wallet dir if we have a failure
+    if (!ret) {
+        fs::remove_all(wallet_path);
+    }
+
+    return ret;
+}

src/wallet/dump.h

@@ -0,0 +1,17 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_WALLET_DUMP_H
+#define BITCOIN_WALLET_DUMP_H
+
+#include <fs.h>
+
+class CWallet;
+
+struct bilingual_str;
+
+bool DumpWallet(CWallet& wallet, bilingual_str& error);
+bool CreateFromDump(const std::string& name, const fs::path& wallet_path, bilingual_str& error, std::vector<bilingual_str>& warnings);
+
+#endif // BITCOIN_WALLET_DUMP_H

src/wallet/wallettool.cpp

@@ -5,6 +5,7 @@
 #include <fs.h>
 #include <util/system.h>
 #include <util/translation.h>
+#include <wallet/dump.h>
 #include <wallet/salvage.h>
 #include <wallet/wallet.h>
 #include <wallet/walletutil.h>
@@ -106,6 +107,17 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)
 {
     fs::path path = fs::absolute(name, GetWalletDir());
 
+    // -format is only allowed with createfromdump. Disallow it for all other commands.
+    if (gArgs.IsArgSet("-format") && command != "createfromdump") {
+        tfm::format(std::cerr, "The -format option can only be used with the \"createfromdump\" command.\n");
+        return false;
+    }
+    // -dumpfile is only allowed with dump and createfromdump. Disallow it for all other commands.
+    if (gArgs.IsArgSet("-dumpfile") && command != "dump" && command != "createfromdump") {
+        tfm::format(std::cerr, "The -dumpfile option can only be used with the \"dump\" and \"createfromdump\" commands.\n");
+        return false;
+    }
+
     if (command == "create") {
         DatabaseOptions options;
         options.require_create = true;
@@ -119,33 +131,55 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)
             WalletShowInfo(wallet_instance.get());
             wallet_instance->Close();
         }
-    } else if (command == "info" || command == "salvage") {
-        if (command == "info") {
-            DatabaseOptions options;
-            options.require_existing = true;
-            std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options);
-            if (!wallet_instance) return false;
-            WalletShowInfo(wallet_instance.get());
-            wallet_instance->Close();
-        } else if (command == "salvage") {
+    } else if (command == "info") {
+        DatabaseOptions options;
+        options.require_existing = true;
+        std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options);
+        if (!wallet_instance) return false;
+        WalletShowInfo(wallet_instance.get());
+        wallet_instance->Close();
+    } else if (command == "salvage") {
 #ifdef USE_BDB
-            bilingual_str error;
-            std::vector<bilingual_str> warnings;
-            bool ret = RecoverDatabaseFile(path, error, warnings);
-            if (!ret) {
-                for (const auto& warning : warnings) {
-                    tfm::format(std::cerr, "%s\n", warning.original);
-                }
-                if (!error.empty()) {
-                    tfm::format(std::cerr, "%s\n", error.original);
-                }
+        bilingual_str error;
+        std::vector<bilingual_str> warnings;
+        bool ret = RecoverDatabaseFile(path, error, warnings);
+        if (!ret) {
+            for (const auto& warning : warnings) {
+                tfm::format(std::cerr, "%s\n", warning.original);
+            }
+            if (!error.empty()) {
+                tfm::format(std::cerr, "%s\n", error.original);
             }
-            return ret;
-#else
-            tfm::format(std::cerr, "Salvage command is not available as BDB support is not compiled");
-            return false;
-#endif
         }
+        return ret;
+#else
+        tfm::format(std::cerr, "Salvage command is not available as BDB support is not compiled");
+        return false;
+#endif
+    } else if (command == "dump") {
+        DatabaseOptions options;
+        options.require_existing = true;
+        std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options);
+        if (!wallet_instance) return false;
+        bilingual_str error;
+        bool ret = DumpWallet(*wallet_instance, error);
+        if (!ret && !error.empty()) {
+            tfm::format(std::cerr, "%s\n", error.original);
+            return ret;
+        }
+        tfm::format(std::cout, "The dumpfile may contain private keys. To ensure the safety of your Bitcoin, do not share the dumpfile.\n");
+        return ret;
+    } else if (command == "createfromdump") {
+        bilingual_str error;
+        std::vector<bilingual_str> warnings;
+        bool ret = CreateFromDump(name, path, error, warnings);
+        for (const auto& warning : warnings) {
+            tfm::format(std::cout, "%s\n", warning.original);
+        }
+        if (!ret && !error.empty()) {
+            tfm::format(std::cerr, "%s\n", error.original);
+        }
+        return ret;
     } else {
         tfm::format(std::cerr, "Invalid command: %s\n", command);
         return false;

test/functional/tool_wallet.py

@@ -10,6 +10,8 @@ import stat
 import subprocess
 import textwrap
 
+from collections import OrderedDict
+
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
@@ -96,6 +98,89 @@ class ToolWalletTest(BitcoinTestFramework):
                 Address Book: %d
             ''' % (wallet_name, keypool, transactions, address * output_types))
 
+    def read_dump(self, filename):
+        dump = OrderedDict()
+        with open(filename, "r", encoding="utf8") as f:
+            for row in f:
+                row = row.strip()
+                key, value = row.split(',')
+                dump[key] = value
+        return dump
+
+    def assert_is_sqlite(self, filename):
+        with open(filename, 'rb') as f:
+            file_magic = f.read(16)
+            assert file_magic == b'SQLite format 3\x00'
+
+    def assert_is_bdb(self, filename):
+        with open(filename, 'rb') as f:
+            f.seek(12, 0)
+            file_magic = f.read(4)
+            assert file_magic == b'\x00\x05\x31\x62' or file_magic == b'\x62\x31\x05\x00'
+
+    def write_dump(self, dump, filename, magic=None, skip_checksum=False):
+        if magic is None:
+            magic = "BITCOIN_CORE_WALLET_DUMP"
+        with open(filename, "w", encoding="utf8") as f:
+            row = ",".join([magic, dump[magic]]) + "\n"
+            f.write(row)
+            for k, v in dump.items():
+                if k == magic or k == "checksum":
+                    continue
+                row = ",".join([k, v]) + "\n"
+                f.write(row)
+            if not skip_checksum:
+                row = ",".join(["checksum", dump["checksum"]]) + "\n"
+                f.write(row)
+
+    def assert_dump(self, expected, received):
+        e = expected.copy()
+        r = received.copy()
+
+        # BDB will add a "version" record that is not present in sqlite
+        # In that case, we should ignore this record in both
+        # But because this also effects the checksum, we also need to drop that.
+        v_key = "0776657273696f6e" # Version key
+        if v_key in e and v_key not in r:
+            del e[v_key]
+            del e["checksum"]
+            del r["checksum"]
+        if v_key not in e and v_key in r:
+            del r[v_key]
+            del e["checksum"]
+            del r["checksum"]
+
+        assert_equal(len(e), len(r))
+        for k, v in e.items():
+            assert_equal(v, r[k])
+
+    def do_tool_createfromdump(self, wallet_name, dumpfile, file_format=None):
+        dumppath = os.path.join(self.nodes[0].datadir, dumpfile)
+        rt_dumppath = os.path.join(self.nodes[0].datadir, "rt-{}.dump".format(wallet_name))
+
+        dump_data = self.read_dump(dumppath)
+
+        args = ["-wallet={}".format(wallet_name),
+                "-dumpfile={}".format(dumppath)]
+        if file_format is not None:
+            args.append("-format={}".format(file_format))
+        args.append("createfromdump")
+
+        load_output = ""
+        if file_format is not None and file_format != dump_data["format"]:
+            load_output += "Warning: Dumpfile wallet format \"{}\" does not match command line specified format \"{}\".\n".format(dump_data["format"], file_format)
+        self.assert_tool_output(load_output, *args)
+        assert os.path.isdir(os.path.join(self.nodes[0].datadir, "regtest/wallets", wallet_name))
+
+        self.assert_tool_output("The dumpfile may contain private keys. To ensure the safety of your Bitcoin, do not share the dumpfile.\n", '-wallet={}'.format(wallet_name), '-dumpfile={}'.format(rt_dumppath), 'dump')
+
+        rt_dump_data = self.read_dump(rt_dumppath)
+        wallet_dat = os.path.join(self.nodes[0].datadir, "regtest/wallets/", wallet_name, "wallet.dat")
+        if rt_dump_data["format"] == "bdb":
+            self.assert_is_bdb(wallet_dat)
+        else:
+            self.assert_is_sqlite(wallet_dat)
+
     def test_invalid_tool_commands_and_args(self):
         self.log.info('Testing that various invalid commands raise with specific error messages')
         self.assert_raises_tool_error('Invalid command: foo', 'foo')
@@ -228,6 +313,81 @@ class ToolWalletTest(BitcoinTestFramework):
 
         self.assert_tool_output('', '-wallet=salvage', 'salvage')
 
+    def test_dump_createfromdump(self):
+        self.start_node(0)
+        self.nodes[0].createwallet("todump")
+        file_format = self.nodes[0].get_wallet_rpc("todump").getwalletinfo()["format"]
+        self.nodes[0].createwallet("todump2")
+        self.stop_node(0)
+
+        self.log.info('Checking dump arguments')
+        self.assert_raises_tool_error('No dump file provided. To use dump, -dumpfile=<filename> must be provided.', '-wallet=todump', 'dump')
+
+        self.log.info('Checking basic dump')
+        wallet_dump = os.path.join(self.nodes[0].datadir, "wallet.dump")
+        self.assert_tool_output('The dumpfile may contain private keys. To ensure the safety of your Bitcoin, do not share the dumpfile.\n', '-wallet=todump', '-dumpfile={}'.format(wallet_dump), 'dump')
+
+        dump_data = self.read_dump(wallet_dump)
+        orig_dump = dump_data.copy()
+        # Check the dump magic
+        assert_equal(dump_data['BITCOIN_CORE_WALLET_DUMP'], '1')
+        # Check the file format
+        assert_equal(dump_data["format"], file_format)
+
+        self.log.info('Checking that a dumpfile cannot be overwritten')
+        self.assert_raises_tool_error('File {} already exists. If you are sure this is what you want, move it out of the way first.'.format(wallet_dump),  '-wallet=todump2', '-dumpfile={}'.format(wallet_dump), 'dump')
+
+        self.log.info('Checking createfromdump arguments')
+        self.assert_raises_tool_error('No dump file provided. To use createfromdump, -dumpfile=<filename> must be provided.', '-wallet=todump', 'createfromdump')
+        non_exist_dump = os.path.join(self.nodes[0].datadir, "wallet.nodump")
+        self.assert_raises_tool_error('Unknown wallet file format "notaformat" provided. Please provide one of "bdb" or "sqlite".', '-wallet=todump', '-format=notaformat', '-dumpfile={}'.format(wallet_dump), 'createfromdump')
+        self.assert_raises_tool_error('Dump file {} does not exist.'.format(non_exist_dump), '-wallet=todump', '-dumpfile={}'.format(non_exist_dump), 'createfromdump')
+        wallet_path = os.path.join(self.nodes[0].datadir, 'regtest/wallets/todump2')
+        self.assert_raises_tool_error('Failed to create database path \'{}\'. Database already exists.'.format(wallet_path), '-wallet=todump2', '-dumpfile={}'.format(wallet_dump), 'createfromdump')
+
+        self.log.info('Checking createfromdump')
+        self.do_tool_createfromdump("load", "wallet.dump")
+        self.do_tool_createfromdump("load-bdb", "wallet.dump", "bdb")
+        self.do_tool_createfromdump("load-sqlite", "wallet.dump", "sqlite")
+
+        self.log.info('Checking createfromdump handling of magic and versions')
+        bad_ver_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_ver1.dump")
+        dump_data["BITCOIN_CORE_WALLET_DUMP"] = "0"
+        self.write_dump(dump_data, bad_ver_wallet_dump)
+        self.assert_raises_tool_error('Error: Dumpfile version is not supported. This version of bitcoin-wallet only supports version 1 dumpfiles. Got dumpfile with version 0', '-wallet=badload', '-dumpfile={}'.format(bad_ver_wallet_dump), 'createfromdump')
+        assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "regtest/wallets", "badload"))
+        bad_ver_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_ver2.dump")
+        dump_data["BITCOIN_CORE_WALLET_DUMP"] = "2"
+        self.write_dump(dump_data, bad_ver_wallet_dump)
+        self.assert_raises_tool_error('Error: Dumpfile version is not supported. This version of bitcoin-wallet only supports version 1 dumpfiles. Got dumpfile with version 2', '-wallet=badload', '-dumpfile={}'.format(bad_ver_wallet_dump), 'createfromdump')
+        assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "regtest/wallets", "badload"))
+        bad_magic_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_magic.dump")
+        del dump_data["BITCOIN_CORE_WALLET_DUMP"]
+        dump_data["not_the_right_magic"] = "1"
+        self.write_dump(dump_data, bad_magic_wallet_dump, "not_the_right_magic")
+        self.assert_raises_tool_error('Error: Dumpfile identifier record is incorrect. Got "not_the_right_magic", expected "BITCOIN_CORE_WALLET_DUMP".', '-wallet=badload', '-dumpfile={}'.format(bad_magic_wallet_dump), 'createfromdump')
+        assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "regtest/wallets", "badload"))
+
+        self.log.info('Checking createfromdump handling of checksums')
+        bad_sum_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_sum1.dump")
+        dump_data = orig_dump.copy()
+        checksum = dump_data["checksum"]
+        dump_data["checksum"] = "1" * 64
+        self.write_dump(dump_data, bad_sum_wallet_dump)
+        self.assert_raises_tool_error('Error: Dumpfile checksum does not match. Computed {}, expected {}'.format(checksum, "1" * 64), '-wallet=bad', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
+        assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "regtest/wallets", "badload"))
+        bad_sum_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_sum2.dump")
+        del dump_data["checksum"]
+        self.write_dump(dump_data, bad_sum_wallet_dump, skip_checksum=True)
+        self.assert_raises_tool_error('Error: Missing checksum', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
+        assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "regtest/wallets", "badload"))
+        bad_sum_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_sum3.dump")
+        dump_data["checksum"] = "2" * 10
+        self.write_dump(dump_data, bad_sum_wallet_dump)
+        self.assert_raises_tool_error('Error: Dumpfile checksum does not match. Computed {}, expected {}{}'.format(checksum, "2" * 10, "0" * 54), '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
+        assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "regtest/wallets", "badload"))
+
+
     def run_test(self):
         self.wallet_path = os.path.join(self.nodes[0].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename)
         self.test_invalid_tool_commands_and_args()
@@ -239,6 +399,7 @@ class ToolWalletTest(BitcoinTestFramework):
         if not self.options.descriptors:
             # Salvage is a legacy wallet only thing
             self.test_salvage()
+        self.test_dump_createfromdump()
 
 if __name__ == '__main__':
     ToolWalletTest().main()