wallet: disallow creating new or restoring to an unnamed (default) wallet

achow101 commented at 11:39 pm on January 12, 2026: member

We’ve been moving in the direction that all wallets must have a name. Therefore, we shouldn’t allow creating new unnamed wallets. createwallet, restorewallet, and the wallet tool’s create and createfromdump all now require the user to provide a non-empty wallet name when creating/restoring a wallet.

The GUI is already enforcing this, but we were not enforcing it for RPCs or in the underlying CreateWallet and RestoreWallet functions.

Wallet migration does still need to be able to restore unnamed wallets, so there is a new argument to RestoreWallet to explicitly allow that behavior for migration only.

DrahtBot added the label Wallet on Jan 12, 2026

DrahtBot commented at 11:39 pm on January 12, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34269.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	rkrux, polespinasa
Stale ACK	Sjors, w0xlt, rogersala21

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34176 (wallet: improve error msg when db directory is not writable by furszy)
#32685 (wallet: Allow read-only database access for info and dump commands by PeterWrighten)
#31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

RestoreWallet(context, backup_path, wallet_name, std::nullopt, status, restore_error, warnings, false, true) in src/wallet/wallet.cpp

2026-01-19

in src/wallet/wallet.cpp:474 in d65c0dad7a

467@@ -461,8 +468,16 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string&
468 
469 // Re-creates wallet from the backup file by renaming and moving it into the wallet's directory.
470 // If 'load_after_restore=true', the wallet object will be fully initialized and appended to the context.
471-std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings, bool load_after_restore)
472+std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings, bool load_after_restore, bool allow_unnamed)
473 {
474+    // Error if the wallet name is empty and allow_unnamed == false
475+    // allow_unnamed == false is only used by migration to migrate an unnamed wallet

Sjors commented at 7:56 am on January 13, 2026:

In d65c0dad7a970d7e99bfea8fb77a2b5ba8f91e9b wallet: disallow unnamed wallets in createwallet and restorewallet: you mean == true?

achow101 commented at 9:22 pm on January 13, 2026:

Fixed

in src/wallet/wallettool.cpp:123 in 5e3d561e62

119     const std::string name = args.GetArg("-wallet", "");
120     const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), fs::PathFromString(name));
121 
122     if (command == "create") {
123+        if (name.empty()) {
124+            tfm::format(std::cerr, "Wallet name cannot be empty");

Sjors commented at 8:03 am on January 13, 2026:

In 5e3d561e624d334916de1ffa1194d2896612c9c0 wallettool: Disallow creating new unnamed wallets: missing newline

achow101 commented at 9:22 pm on January 13, 2026:

Done

in src/wallet/rpc/util.cpp:149 in d65c0dad7a outdated

141@@ -142,9 +142,13 @@ void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& st
142             case DatabaseStatus::FAILED_ALREADY_EXISTS:
143                 code = RPC_WALLET_ALREADY_EXISTS;
144                 break;
145+            case DatabaseStatus::FAILED_NEW_UNNAMED:
146             case DatabaseStatus::FAILED_INVALID_BACKUP_FILE:
147                 code = RPC_INVALID_PARAMETER;
148                 break;
149+            case DatabaseStatus::FAILED_ENCRYPT:

Sjors commented at 8:05 am on January 13, 2026:

In d65c0dad7a970d7e99bfea8fb77a2b5ba8f91e9b wallet: disallow unnamed wallets in createwallet and restorewallet nit: maybe do this is a prep commit.

polespinasa commented at 4:59 pm on January 13, 2026:

I fail to see what the change on DatabaseStatus::FAILED_ENCRYPT does related to this PR. Is it just a wrong error code handling unrelated to the PR?

achow101 commented at 9:23 pm on January 13, 2026:

maybe do this is a prep commit.

Done

I fail to see what the change on DatabaseStatus::FAILED_ENCRYPT does related to this PR. Is it just a wrong error code handling unrelated to the PR?

This was to allow HandleWalletError to also be used in createwallet while still preserving the error code it would throw for FAILED_ENCRYPT.

polespinasa commented at 7:55 am on January 14, 2026:

Oh I see I missed the ternary operation, thx :)

in test/functional/test_framework/test_node.py:912 in d65c0dad7a

907@@ -907,6 +908,13 @@ def bumpmocktime(self, seconds):
908     def wait_until(self, test_function, timeout=60, check_interval=0.05):
909         return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.timeout_factor, check_interval=check_interval)
910 
911+    def create_unnamed_wallet(self, **kwargs):
912+        """Create an unnamed wallet on this node since createwallet explicitly disallows that"""

Sjors commented at 8:13 am on January 13, 2026:

In d65c0dad7a970d7e99bfea8fb77a2b5ba8f91e9b wallet: disallow unnamed wallets in createwallet and restorewallet:

0"""createwallet disallows empty wallet names, so create a temporary
1named wallet and move its wallet.dat to the legacy unnamed-wallet
2location."""

achow101 commented at 9:23 pm on January 13, 2026:

Done

in src/wallet/wallettool.cpp:170 in 5e3d561e62

166@@ -163,6 +167,10 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
167         tfm::format(std::cout, "The dumpfile may contain private keys. To ensure the safety of your Bitcoin, do not share the dumpfile.\n");
168         return ret;
169     } else if (command == "createfromdump") {
170+        if (name.empty()) {

Sjors commented at 8:28 am on January 13, 2026:

In 5e3d561e624d334916de1ffa1194d2896612c9c0 wallettool: Disallow creating new unnamed wallets: why not do this check inside CreateFromDump? That seems a bit safer against regressions if that function ever gets more than one call site.

achow101 commented at 9:23 pm on January 13, 2026:

Done

Sjors commented at 8:31 am on January 13, 2026: member

Concept ACK

Mostly happy with 5e3d561e624d334916de1ffa1194d2896612c9c0. The missing newline issue needs fixing, the rest is more cosmetic.

in test/functional/wallet_startup.py:33 in d65c0dad7a

29@@ -30,7 +30,7 @@ def run_test(self):
30         assert_equal(self.nodes[0].listwalletdir(), {'wallets': []})
31 
32         self.log.info('New default wallet should load by default when there are no other wallets')
33-        self.nodes[0].createwallet(wallet_name='', load_on_startup=False)
34+        self.nodes[0].create_unnamed_wallet(load_on_startup=False)

polespinasa commented at 5:20 pm on January 13, 2026:

why no just add a name to the wallet?

achow101 commented at 9:21 pm on January 13, 2026:

This test is specifically regarding the unnamed wallet (referred to as the default wallet) and loading it automatically on startup if it’s there by itself.

polespinasa commented at 7:57 am on January 14, 2026:

Maybe worth a comment to make it easy to understand if you lack context?

Sjors commented at 10:22 am on January 14, 2026:

I think there’s enough context if you hover over the create_unnamed_wallet helper? Or is there something else about this test that needs clarifying?

polespinasa commented at 1:56 pm on January 14, 2026:

Yes what create_unnamed_wallet does is clear, what is not clear is that the test needs an unnamed wallet. Because without context it is not clear that default wallet means unnamed.

That lack of context is what made me ask why not just add a name to the wallet.

Something like this might be enough:

0self.log.info('New default (unnamed) wallet should load by default when there are no other wallets')

achow101 commented at 6:51 pm on January 14, 2026:

Because without context it is not clear that default wallet means unnamed.

Default wallet has always meant the unnamed wallet.

in test/functional/test_framework/test_node.py:911 in d65c0dad7a

907@@ -907,6 +908,13 @@ def bumpmocktime(self, seconds):
908     def wait_until(self, test_function, timeout=60, check_interval=0.05):
909         return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.timeout_factor, check_interval=check_interval)
910 
911+    def create_unnamed_wallet(self, **kwargs):

polespinasa commented at 5:22 pm on January 13, 2026:

this is only used in wallet_startup.py and I fail to see why that wallet must be unnamed. Why do we need this function?

achow101 commented at 9:24 pm on January 13, 2026:

I was expecting to need this function in other tests, but that turned out to not be necessary. Moved it back into wallet_startup.py.

polespinasa commented at 5:25 pm on January 13, 2026: member

concept ACK

reviewed first commit d65c0dad7a970d7e99bfea8fb77a2b5ba8f91e9b will review the second one later.

Left some questions.

wallet, rpc: Use HandleWalletError in createwallet

Use the utility function HandleWalletError to deal with wallet creation
errors in createwallet.

d30ad4a912

achow101 force-pushed on Jan 13, 2026

polespinasa approved

polespinasa commented at 8:09 am on January 14, 2026: member

code lgtm tested ACK 1261089b1d2216e04c16803fe1b913cbeb9afd9b

Prior to this PR you could create or restore a wallet using an empty name "", now you cannot and it fails returning an error code.

0$ ./build/bin/bitcoin-cli createwallet ""
1error code: -8
2error message:
3Wallet name cannot be empty

0$ ./build/bin/bitcoin-wallet -wallet='' create
1Wallet name cannot be empty

DrahtBot requested review from Sjors on Jan 14, 2026

Sjors commented at 10:32 am on January 14, 2026: member

utACK 1261089b1d2216e04c16803fe1b913cbeb9afd9b

in src/wallet/wallettool.cpp:122 in 1261089b1d outdated

118     }
119     const std::string name = args.GetArg("-wallet", "");
120     const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), fs::PathFromString(name));
121 
122     if (command == "create") {
123+        if (name.empty()) {

w0xlt commented at 6:29 pm on January 14, 2026:

Is this code reachable considering the condition already being checked above if ((command == "create" || command == "createfromdump") && !args.IsArgSet("-wallet")) { ?

achow101 commented at 6:52 pm on January 14, 2026:

Yes, -wallet= is a valid way to set -wallet

w0xlt commented at 6:33 pm on January 14, 2026: contributor

lgtm ACK https://github.com/bitcoin/bitcoin/pull/34269/commits/1261089b1d2216e04c16803fe1b913cbeb9afd9b with the above minor nit

rogersala21 commented at 10:01 pm on January 17, 2026: none

ACK 1261089b1d2216e04c16803fe1b913cbeb9afd9b

Tested and reviewed the code lgtm.

Now restorewallet does not work with empty name "".

0$ /bitcoin-cli restorewallet "" walletbackup.dat 
1error code: -8
2error message:
3Wallet name cannot be empty

By the way, should we consider disallowing wallet names that consist only of whitespace/s? They’re non-empty but effectively indistinguishable from unnamed wallets from a UX perspective, which could be confusing.

in test/functional/wallet_startup.py:30 in 57cadc60c8 outdated

26@@ -24,13 +27,24 @@ def setup_nodes(self):
27         self.add_nodes(self.num_nodes)
28         self.start_nodes()
29 
30+    def create_unnamed_wallet(self, **kwargs):

rkrux commented at 9:11 am on January 19, 2026:

In 57cadc60c847f2c3869e7a12c92b718e0c3e6524 “wallet: disallow unnamed wallets in createwallet and restorewallet”

Nit: To make it clear in the tests as well that we can’t create unnamed wallets naturally.

0- def create_unnamed_wallet(self, **kwargs):
1+ def create_synthetic_unnamed_wallet(self, **kwargs):

achow101 commented at 7:12 pm on January 19, 2026:

I don’t think that’s necessary. The comment on the function also explains this.

in src/wallet/wallet.h:1 in 57cadc60c8 outdated

rkrux commented at 9:13 am on January 19, 2026:

In 57cadc6 “wallet: disallow unnamed wallets in createwallet and restorewallet”

Wallet migration does still need to be able to restore unnamed wallets, so there is a new argument to RestoreWallet to explicitly allow that behavior for migration only.

Nit: This note from the PR description can be present in this commit message as well.

rkrux commented at 9:38 am on January 19, 2026:

In 1261089b1d2216e04c16803fe1b913cbeb9afd9b “wallettool: Disallow creating new unnamed wallets”

 0diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py
 1index 4f764c801e..9255b26166 100755
 2--- a/test/functional/tool_wallet.py
 3+++ b/test/functional/tool_wallet.py
 4@@ -136,6 +136,7 @@ class ToolWalletTest(BitcoinTestFramework):
 5         self.assert_raises_tool_error('Error parsing command line arguments: Invalid parameter -foo', '-foo')
 6         self.assert_raises_tool_error('No method provided. Run `bitcoin-wallet -help` for valid methods.')
 7         self.assert_raises_tool_error('Wallet name must be provided when creating a new wallet.', 'create')
 8+        self.assert_raises_tool_error('Wallet name must be provided when creating a new wallet.', 'createfromdump')
 9         error = f"SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of {self.config['environment']['CLIENT_NAME']}?"
10         self.assert_raises_tool_error(
11             error,
12(END)

achow101 commented at 7:12 pm on January 19, 2026:

Done

achow101 commented at 7:13 pm on January 19, 2026:

Done

rkrux commented at 9:39 am on January 19, 2026: contributor

Looks fine at this stage 1261089b1d2216e04c16803fe1b913cbeb9afd9b.

Left couple of nits and a test case.

wallet: disallow unnamed wallets in createwallet and restorewallet

Migration still needs to be able to restore unnamed wallets, so
allow_unnamed is added to RestoreWallet to explicitly allow that
behavior for migration only.

5875a9c502

wallettool: Disallow creating new unnamed wallets 75b704df9d

achow101 force-pushed on Jan 19, 2026

rkrux commented at 1:46 pm on January 20, 2026: contributor

lgtm ACK 75b704df9d5ccb262d19ffe95b7ca6d1a934f3fb

0git range-diff 1261089...75b704d

DrahtBot requested review from w0xlt on Jan 20, 2026

DrahtBot requested review from rogersala21 on Jan 20, 2026

DrahtBot requested review from polespinasa on Jan 20, 2026

polespinasa commented at 2:59 pm on January 20, 2026: member

re ACK 75b704df9d5ccb262d19ffe95b7ca6d1a934f3fb

sedited merged this on Jan 21, 2026

sedited closed this on Jan 21, 2026

wallet: disallow creating new or restoring to an unnamed (default) wallet #34269

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)