Remove the automatic creation and loading of the default wallet

achow101 commented at 10:30 pm on February 20, 2019: member

Instead of automatically creating and loading a default wallet, users should instead explicitly create their wallet or load it on start.

Builds on #19754 which provides the load_on_startup behavior for the GUI.

fanquake added the label Wallet on Feb 20, 2019

DrahtBot commented at 11:33 pm on February 20, 2019: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#18836 (wallet: upgradewallet fixes and additional tests by achow101)

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.

in src/wallet/init.cpp:119 in 5a9ccf2910 outdated

183@@ -184,8 +184,16 @@ void WalletInit::Construct(InitInterfaces& interfaces) const
184         LogPrintf("Wallet disabled!\n");
185         return;
186     }
187-    gArgs.SoftSetArg("-wallet", "");

promag commented at 11:58 pm on February 20, 2019:

Unless I’m missing something, this is breaking change. Why not change the behavior from “load or create each -wallet” to “just load each -wallet”? Then to create a wallet there is the option in the GUI and createwallet in RPC.

DrahtBot added the label Needs rebase on Feb 21, 2019

Sjors commented at 9:20 am on February 21, 2019: member

Partial Concept ACK on:

not creating a default wallet with bitcoind
storing opened wallets in bitcoin_rw.conf (QT only)

Concept On The Fence about making this a breaking change for bitcoind. I.e. maybe we should still load an existing <datadir>/wallet.dat and <datadir>/wallet.dat, but not create one.

Concept NACK on:

having QT start with a blank wallet, where the user is instructed to go the file menu and choose Create or Open Wallet
not loading the default wallet in QT after upgrading: that’s a guarantee for user heart attacks

I think the best solution is here is to pull the create wallet UI into the startup wizard, as an additional section. The user can then just click next and it creates the default wallet.

Since it takes at least a few seconds, this gives the RNG a bit more bit time too (I assume the IBD network traffic should generate some extra entropy). (Although that requires starting IBD while the wizard is open.)

An easier but still acceptable solution is a big Create Wallet button in the Overview tab (when no wallet is loaded), as well as point out that they can load an existing wallet (less likely for first time users).

Some issues:

opening and closing wallets in QT updates bitcoin_rw.conf, which is good but:
- why use a new field loadedwallet? adding and removing wallet= entries seems cleaner. It won’t work for wallets already in bitcoin.conf, but that’s fine (we can warn the user in that case).
- I suggest doing this as soon as the open / close succeeds, rather than at exit

achow101 force-pushed on Feb 21, 2019

achow101 commented at 6:45 pm on February 21, 2019: member

An easier but still acceptable solution is a big Create Wallet button in the Overview tab (when no wallet is loaded), as well as point out that they can load an existing wallet (less likely for first time users).

I’ve done this.

not loading the default wallet in QT after upgrading: that’s a guarantee for user heart attacks

I’ve changed that back to the original behavior except that it only loads the default wallet if it is available. It will not create it.

why use a new field loadedwallet? adding and removing wallet= entries seems cleaner. It won’t work for wallets already in bitcoin.conf, but that’s fine (we can warn the user in that case).

#11082 doesn’t let me set the multiple of the same argument.

Sjors commented at 9:04 am on February 22, 2019: member

Much better, thanks. Both bitcoind and QT now load the default wallet if it exists, and the QT create button is more clear.

Can you add screenshots of the UX to the PR description?

The initial download overlay actually hides the wallet functionality, which has the nice side-effect of collecting some entropy. Although I’d rather work on making the initial experience such that users can get started more quickly, that’s a whole different story.

When the user hides the sync they see this:

A bit ugly, but I think the button makes it super clear what to do. Nit: reverse the order, i.e. suggest creating a wallet and then point out that the user can also load an existing wallet.

Regarding the wallet creation dialog, that can be discussed in #15450, but it’s important that we don’t overwhelm a first time user with advanced options. One thing though: what to do with the default name, at the risk of bike shedding…

I suggest we hide the wallet name field for the first wallet, or make it really clear that it’s totally optional and that you can’t (easily) change it (yet).

As for rw_config, which has been in limbo for quite a long time, let’s start with reviewing #14866 by @AkioNak which makes the evaluation of bitcoin.conf (includes) a bit more sane. We should expand the functional test suite to more thoroughly cover the expected behavior. That in turn should make it easier to merge rw_config.

Found a small bug:

0src/bitcoin-cli -testnet listwallets
1[
2]
3
4src/bitcoin-cli -testnet createwallet ""
5error code: -4
6error message:
7Wallet  already exists

jnewbery commented at 1:38 pm on February 22, 2019: member

concept ACK

in src/wallet/init.cpp:243 in 0c61dd9e71 outdated

239@@ -228,10 +240,19 @@ void StopWallets()
240 void UnloadWallets()
241 {
242     auto wallets = GetWallets();
243+    std::string loaded_wallets = "";

practicalswift commented at 11:32 pm on February 25, 2019:

Nit: Redundant initialisation :-)

jnewbery added the label Revise OP on Jun 7, 2019

achow101 force-pushed on Oct 16, 2019

DrahtBot removed the label Needs rebase on Oct 16, 2019

DrahtBot added the label Needs rebase on Oct 18, 2019

achow101 force-pushed on Oct 21, 2019

DrahtBot removed the label Needs rebase on Oct 21, 2019

in src/wallet/load.cpp:120 in 4f404be069 outdated

115+
116+        // Write the wallet name to rw conf
117+        if (!loaded_wallets.empty()) {
118+            loaded_wallets += ",";
119+        }
120+        loaded_wallets += wallet->GetName();

luke-jr commented at 8:26 pm on October 21, 2019:

What if the wallet name has a comma? Would prefer to see rwconf extended to multiple wallet= fields…

achow101 commented at 9:10 pm on October 21, 2019:

So would I. I would prefer that #11082 implements it that way instead of requiring me to change it as I am not familiar with that code.

in src/wallet/load.cpp:127 in 4f404be069 outdated

122         wallets.pop_back();
123         RemoveWallet(wallet);
124         UnloadWallet(std::move(wallet));
125     }
126+    if (gArgs.HaveRWConfigFile()) {
127+        gArgs.ModifyRWConfigFile("loadedwallet", loaded_wallets);

luke-jr commented at 8:26 pm on October 21, 2019:

Should just use wallet=?

achow101 commented at 9:11 pm on October 21, 2019:

That will cause some problems as is since it would be one string that represents multiple wallets. If/when rwconf allows repeated fields, then it can be wallet=.

jnewbery removed the label Revise OP on Oct 21, 2019

achow101 force-pushed on Oct 21, 2019

achow101 force-pushed on Oct 23, 2019

DrahtBot added the label Needs rebase on Oct 30, 2019

achow101 force-pushed on Oct 31, 2019

DrahtBot removed the label Needs rebase on Oct 31, 2019

DrahtBot added the label Needs rebase on Nov 8, 2019

achow101 force-pushed on Jun 22, 2020

achow101 commented at 10:46 pm on June 22, 2020: member

Rebased this on top of #15937

DrahtBot removed the label Needs rebase on Jun 22, 2020

in src/wallet/init.cpp:121 in 6feed9508a outdated

115@@ -116,6 +116,9 @@ void WalletInit::Construct(NodeContext& node) const
116         LogPrintf("Wallet disabled!\n");
117         return;
118     }
119-    gArgs.SoftSetArg("-wallet", "");
120+    WalletLocation default_wallet("wallet.dat");
121+    if (default_wallet.Exists()) {
122+        gArgs.SoftSetArg("-wallet", "");

ryanofsky commented at 1:31 pm on June 25, 2020:

In commit “Do not create default wallet” (6feed9508a225276cc2264b7d19b151a62deecea)

Was experimenting with this PR and would suggest the following fixup: 973bb7e652c4ce10ee536e9967a88c80f6dbe2e0 (tag). SoftSet doesn’t really work here for a persistent setting. I’ll backport this fixup to #15937 so the diff here will be smaller, and the only code change this should have to make is adding && WalletLocation("").Exists()

ryanofsky commented at 9:54 pm on June 25, 2020:

re: #15454 (review)

Was experimenting with this PR and would suggest the following fixup: 973bb7e (tag)

Feel free to steal updated version of this PR with fix above squashed and newer changes from 15937: https://github.com/ryanofsky/bitcoin/commits/review.15454.2-edit

ryanofsky approved

ryanofsky commented at 1:38 pm on June 25, 2020: member

Approach ACK 830351b6531eb532830cc6f6523232f16b8958d1. Simple & good change overall

achow101 force-pushed on Jun 25, 2020

achow101 commented at 10:30 pm on June 25, 2020: member

Updated with the changes from #15937

DrahtBot added the label Needs rebase on Jul 1, 2020

Sjors commented at 7:24 pm on July 2, 2020: member

Could use a fresh rebase in light of discussion in bitcoin-core/gui#26

achow101 commented at 7:30 pm on July 2, 2020: member

Could use a fresh rebase in light of discussion in bitcoin-core/gui#26

Waiting on #15937 rebase

achow101 force-pushed on Jul 6, 2020

DrahtBot removed the label Needs rebase on Jul 6, 2020

DrahtBot added the label Needs rebase on Jul 11, 2020

achow101 force-pushed on Jul 11, 2020

ryanofsky approved

ryanofsky commented at 8:51 pm on July 21, 2020: member

Code review ACK efd93dec07906b4f28e0a9b5620c2c426c9e80d2 last 3 commits. No changes since last review other than rebase

achow101 force-pushed on Jul 27, 2020

DrahtBot removed the label Needs rebase on Jul 27, 2020

DrahtBot added the label Needs rebase on Aug 7, 2020

achow101 force-pushed on Aug 10, 2020

DrahtBot removed the label Needs rebase on Aug 10, 2020

achow101 force-pushed on Aug 11, 2020

ryanofsky approved

ryanofsky commented at 9:42 pm on August 11, 2020: member

Code review ACK 546bdce4ece60635a1eb2fa9d46d82ae41aeee24 last 3 commits. No changes since last review other than rebase

DrahtBot added the label Needs rebase on Aug 13, 2020

in src/qt/walletframe.cpp:37 in 546bdce4ec outdated

29@@ -25,9 +30,25 @@ WalletFrame::WalletFrame(const PlatformStyle *_platformStyle, BitcoinGUI *_gui)
30     walletFrameLayout->setContentsMargins(0,0,0,0);
31     walletFrameLayout->addWidget(walletStack);
32 
33-    QLabel *noWallet = new QLabel(tr("No wallet has been loaded."));
34+    // hbox for no wallet
35+    QGroupBox* no_wallet_group = new QGroupBox(walletStack);
36+    QVBoxLayout* no_wallet_layout = new QVBoxLayout(no_wallet_group);
37+
38+    QLabel *noWallet = new QLabel(tr("No wallet has been loaded.\nGo to File > Open Wallet to load a wallet.\n- OR -"));

flack commented at 7:12 pm on August 13, 2020:

wouldn’t it be nicer to just make a open_wallet_button here like it is done below for the create button? That would save the user a bit of mousing around. Also, it would remove the headache of making sure that File > Open Wallet stays consistent with how the menu items are actually called

achow101 commented at 7:59 pm on August 13, 2020:

It would, but the way that opening wallets works now would require more extensive changes with code that I’m not too familiar with.

Someone who is more familiar with the GUI can make this nicer in the future.

flack commented at 10:07 pm on August 13, 2020:

if this were Javascript, I would simply make a button that clicks the menu item. But I guess that won’t fly in Qt :-)

achow101 commented at 10:31 pm on August 13, 2020:

The menu item doesn’t open a selection dialog, it opens a submenu listing all of the wallets. That doesn’t really work here.

promag commented at 2:11 pm on August 14, 2020:

Do you think we should replace the submenu with a dialog?

achow101 commented at 3:33 pm on August 14, 2020:

I suppose this could just be a dropdown listing all of the available wallets. But, as I said, I leave this up to someone more familiar with the GUI to do.

flack commented at 3:45 pm on August 14, 2020:

if it were up to me I’d say a simple filepicker would totally suffice. It’s self-explanatory and flexible, and the more fancy stuff is still available from the menu for those who want it

achow101 commented at 3:52 pm on August 14, 2020:

We would need #15204 to do a file picker type thing.

flack commented at 3:55 pm on August 14, 2020:

ah, sorry, I somehow thought that was already merged

promag commented at 11:47 pm on August 14, 2020:

@flack see #15204 (comment).

achow101 force-pushed on Aug 13, 2020

DrahtBot removed the label Needs rebase on Aug 13, 2020

achow101 force-pushed on Aug 15, 2020

achow101 force-pushed on Aug 17, 2020

achow101 commented at 8:36 pm on August 17, 2020: member

I’ve changed the way that the GUI handles the load_on_start stuff and split it into #19754

achow101 force-pushed on Aug 17, 2020

Rspigler commented at 5:50 pm on August 22, 2020: contributor

@achow101 I know you mentioned #11082 a couple times in the writing of this. Does the upcoming closing of that PR, and the merging of the alternative #15935 affect this?

achow101 commented at 8:42 pm on August 22, 2020: member

@Rspigler this PR had since been changed to use load_on_start so #11082 is not needed.

ryanofsky approved

ryanofsky commented at 10:22 pm on August 24, 2020: member

Code review ACK a386fcd605509767b20a90719cf966e093bfa5dd. Only change since last review is rebasing and replacing bbda06f9cd1345aa80301bfc98b1cae4ec231f6d with e8bfa09b1c1888c33acfd5dcebbe136bf0d6fe82 from #19754.

re: #15454#issue-254819021

Instead of automatically creating and loading a default wallet, users should instead explicitly create their wallet or load it on start.

This makes use of the load_on_start feature provided by #15937. The GUI is also changed to always do load on start for all loaded and created wallets.

It would probably be good to update PR description to mention #19754 instead of #15937

DrahtBot added the label Needs rebase on Aug 31, 2020

achow101 force-pushed on Aug 31, 2020

Sjors commented at 6:29 pm on August 31, 2020: member

Having https://github.com/bitcoin-core/gui/issues/77 implemented would go a long way towards making the “Create Wallet” dialog - which all new users have to deal with after this PR - less intimidating.

DrahtBot removed the label Needs rebase on Aug 31, 2020

ryanofsky commented at 0:13 am on September 2, 2020: member

#19754 isn’t really mentioned anywhere here but the first commit in this PR is from #19754 and can be reviewed there

ryanofsky approved

ryanofsky commented at 0:24 am on September 2, 2020: member

Code review ACK 952bf2c9eef9d6b2f1a01142191c5f41403ed3de. No changes since last review except in first commit which is its own PR #19754

promag commented at 0:56 am on September 3, 2020: member

I guess we can leave #19754 (comment) for another PR. But I wonder what the best approach would be. Currently we have:

0auto wallet_client = interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"));

Which then calls

0std::shared_ptr<CWallet> pwallet = CWallet::CreateWalletFromFile(chain, WalletLocation(walletFile), error, warnings);

So, for this code path, we should add some sort of flag to “create if exists”/“just load”.

Alternatively we could filter settings -wallet, but I think that’s the best approach.

ryanofsky commented at 2:26 am on September 3, 2020: member

I guess we can leave #19754 (comment) for another PR. But I wonder what the best approach would be. Currently we have:

Ideally, I don’t think it should be handled in the wallet/init.cpp Construct method. That method was only meant to construct WalletClient object early on startup, not to access the filesystem or do anything slow or blocking. Also with #10102, that function runs in the node process, not the wallet process.

More ideally, handling not existing wallets would be done in wallet/load.cpp VerifyWallets function. With #19619 all that would be required is setting the DatabaseOptions::require_existing option and handling DatabaseStatus::FAILED_NOT_FOUND return code. It would be easy to implement by permanently removing non-existing wallets from settings.json there, or by just temporarily ignoring them.

More difficult but nicer, I think, would be to have non-existing wallets not removed from settings.json, and shown in the GUI in a disabled state, grayed out with “Wallet path was not accessible” message. That way user could be informed that wallet wasn’t accessible and decide whether or not they want to keep trying to load it in the future.

promag commented at 2:32 am on September 3, 2020: member

and shown in the GUI in a disabled state

IMO more appropriate is to show a modal dialog on start (for each non existing wallet) asking if it should be removed/forgotten from settings, and if he chooses to keep it then it could be disabled.

ryanofsky commented at 2:41 am on September 3, 2020: member

Matter of taste, I guess. I don’t love modal dialogs, and if I have a multiple wallets and some of them are on an encrypted mount or external drive which is unavailable, I would rather just see the wallets listed normally, disabled and unavailable, not have to manually click through a bunch of pop-up dialogs confirming serially that I still want to load each wallet in the future.

promag commented at 2:43 am on September 3, 2020: member

How would you drop disabled wallets (from settings)?

ryanofsky commented at 2:47 am on September 3, 2020: member

How would you drop disabled wallets (from settings)?

The same way you drop an enabled wallet from settings. Switch to wallet in drop-down, go to File -> Close Wallet…

promag commented at 2:51 am on September 3, 2020: member

Oh, so it would be possible to switch in the combo (not actually disabled) but then WalletFrame/WalletView should say that wallet is not available or something like that and probably disable send/receive/transactions views.

ryanofsky commented at 2:59 am on September 3, 2020: member

Yes, exactly. I guess the term “disabled” is overloaded. But that’s just a suggestion. Other approaches (just silently not loading non-existing wallets, silently not loading and also removing from settings, or prompting the user) all seem reasonable and probably easier to implement.

DrahtBot added the label Needs rebase on Sep 3, 2020

achow101 force-pushed on Sep 3, 2020

DrahtBot removed the label Needs rebase on Sep 3, 2020

meshcollider commented at 3:34 am on September 7, 2020: contributor

Error building after #19619 was merged:

0wallet/init.cpp: In member function ‘virtual void WalletInit::Construct(NodeContext&) const’:
1wallet/init.cpp:112:37: error: ‘WalletLocation’ was not declared in this scope
2     if (!args.IsArgSet("wallet") && WalletLocation("").Exists()) {
3                                     ^~~~~~~~~~~~~~
4wallet/init.cpp:112:37: note: suggested alternative: ‘WalletBatch’
5     if (!args.IsArgSet("wallet") && WalletLocation("").Exists()) {
6                                     ^~~~~~~~~~~~~~
7                                     WalletBatch
8Makefile:11482: recipe for target 'wallet/libbitcoin_server_a-init.o' failed

achow101 commented at 5:34 am on September 7, 2020: member

Rebased

achow101 force-pushed on Sep 7, 2020

meshcollider commented at 1:33 am on September 8, 2020: contributor

Lightly tested ACK c24bb1a09cc608b499fefba2402a497cf504217d

ryanofsky approved

ryanofsky commented at 11:20 pm on September 8, 2020: member

Code review ACK c24bb1a09cc608b499fefba2402a497cf504217d, but I think it would be good to fix up the first commit to not do file I/O in WalletInit::Construct and not hardcode “wallet.dat” berkeley wallet name outside of bdb.cpp, and to call existing MakeWalletDatabase function instead of adding a new HasDefaultWallet function. Suggested changes that I think would be to squash are: 421a5502ec71dff3e2d26c7f63cbd3d8da10a706 (branch) +32/-33 lines.

Also would also be good if the change had release notes. Off the top of my head something like “Bitcoin Core will no longer create an unnamed "" wallet by default when no -wallet=<name> command line or wallet=<name> configuration options are used, but for backwards compatibility if a "" wallet already exists and would have been loaded previously, it will still be loaded. Instead of automatically creating an unnamed unencrypted wallet, the GUI now prompts to create a named, encrypted wallet. Instead of only loading the unnamed "" wallet on startup the GUI loads previously loaded wallets.”

Do not create default wallet

No longer create a default wallet. The default wallet will still be
loaded if it exists and not other wallets were specified (anywhere,
including settings.json, bitcoin.conf, and command line).

Tests are updated to be started with -wallet= if they need the default
wallet.

Added test to wallet_startup.py testing that no default wallet is
created and that it is loaded if it exists and no other wallets were
specified.

1bee1e6269

Tell users how to load or create a wallet when no wallet is loaded d26f0648f1

achow101 force-pushed on Sep 9, 2020

achow101 commented at 1:03 am on September 9, 2020: member

Squashed in @ryanofsky’s commit. Also added release notes.

ryanofsky approved

ryanofsky commented at 1:45 am on September 9, 2020: member

Code review ACK d26f0648f1c0d1115dcb8d76e57195032b88f400. Just suggested changes to first commit (reusing MakeWalletDatabase and adding release notes), no changes to second commit

ryanofsky commented at 2:21 pm on September 15, 2020: member

I would really like to see this merged, because I think it’s a significant interface change (or at least a notable one) that a lot of other changes can build on:

GUI improvements like https://github.com/bitcoin-core/gui/issues/77
Fixing bad behavior of settings.json wallet list recreating removed wallets #19754 (comment)
Maybe turning off non-explicit wallet create entirely, and making wallet creation require a createwallet call or gui interaction. jb55 reported problems caused by this yesterday in IRC: http://www.erisian.com.au/bitcoin-core-dev/log-2020-09-14.html#l-355
Test improvements: getting rid of -wallet/createwallet RPC override hacks and "" wallet reliance in in test framework, and switching to explicit named creates. 5d2e85c5d33c3b65c79673cad5d7bc0e5875b107 starts down this road

jonatack commented at 2:25 pm on September 15, 2020: member

Will review soon.

instagibbs commented at 5:00 pm on September 16, 2020: member

also publicly committing to reviewing soon

jnewbery commented at 10:49 am on September 17, 2020: member

Manual test and very light code review ACK d26f0648f1c0d1115dcb8d76e57195032b88f400

I manually tested:

default wallet is loaded on startup, even if “load_on_startup” is not set for the default wallet.
if no default wallet exists, it’s not created on startup
this doesn’t effect whether other wallets are loaded on startup or not.

jonatack commented at 12:33 pm on September 17, 2020: member

ACK d26f0648f1c0d1115dcb8d76e57195032b88f400 light code review, debug build, ran tests, did manual testing with testnet, rebased on master, on linux debian.

One edge case seen: If I load a wallet (default or no), shutdown without unloading it, then rm wallet.dat or rm -rf the wallet folder, then restart, a new wallet is created in its place.

Screenshot from 2020-09-17 14-51-15

jonatack commented at 12:52 pm on September 17, 2020: member

It seems possible to remove the additional “-wallet=” extra_arg from a number of the changed tests before finding a couple that fail without it. Passing -wallet without a value also seems fine:

0-            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
1-            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
2-            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
3-            ["-whitelist=noban@127.0.0.1", "-wallet="],
4+            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet"],
5+            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet"],
6+            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet"],
7+            ["-whitelist=noban@127.0.0.1", "-wallet"],

in src/wallet/load.cpp:46 in d26f0648f1

40@@ -41,10 +41,27 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
41 
42     chain.initMessage(_("Verifying wallet(s)...").translated);
43 
44+    // For backwards compatibility if an unnamed top level wallet exists in the
45+    // wallets directory, include it in the default list of wallets to load.
46+    if (!gArgs.IsArgSet("wallet")) {

jonatack commented at 1:00 pm on September 17, 2020:

Would it better to do:

0-    if (!gArgs.IsArgSet("wallet")) {
1+    if (!(gArgs.GetBoolArg("wallet", false)) {

ryanofsky commented at 1:04 pm on September 17, 2020:

re: #15454 (review)

Would it better to do:

0-    if (!gArgs.IsArgSet("wallet")) {
1+    if (!(gArgs.GetBoolArg("wallet", false)) {

This would be bad because it would cause a -nowallet setting to load the default wallet.

ryanofsky commented at 1:15 pm on September 17, 2020: member

re: #15454 (comment)

Passing -wallet without a value also seems fine?

Would not suggest this because that syntax more typically is used to set a boolean setting to true than to set a string setting to empty. #16545 would actually make it an error to use -string syntax for future string-only settings and would require -string= or -string=""

I think both versions of:

0-            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
1+            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet"],

are less clear than they could be, which is why the the followup https://github.com/ryanofsky/bitcoin/commit/5d2e85c5d33c3b65c79673cad5d7bc0e5875b107 I mentioned in #15454 (comment) would change these to:

0             ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet=" + self.default_wallet_name],

in doc/release-notes-15454.md:4 in 1bee1e6269 outdated

0@@ -0,0 +1,6 @@
1+Wallet
2+------
3+
4+Bitcoin Core will no longer create an unnamed `""` wallet by default when no wallet is specified on the command line or in the configuration files.

instagibbs commented at 5:08 pm on September 17, 2020:

Bitcoin Core will no longer create an unnamed "" wallet by default.

Just stop here. It won’t make a default wallet of any kind, whether or not you specify one specifically.

achow101 commented at 5:09 pm on September 17, 2020:

I think you can make an unnamed wallet with createwallet

instagibbs commented at 5:12 pm on September 17, 2020:

by default

is the key phrase here. It makes nothing by default.

achow101 commented at 5:15 pm on September 17, 2020:

Meh. Release notes can always be edited when they go up on the dev wiki.

instagibbs commented at 5:19 pm on September 17, 2020:

Ok we’ll just hope our readers are deficient in English ;P

flack commented at 5:23 pm on September 17, 2020:

maybe just leave out “by default”?

meshcollider commented at 11:42 pm on September 17, 2020: contributor

This is fine to go in as-is, as mentioned, we can easily make changes to the release notes when they are collated.

meshcollider merged this on Sep 18, 2020

meshcollider closed this on Sep 18, 2020

Sjors commented at 8:25 am on September 18, 2020: member

I made some changes to the wallet creation dialog, because I don’t think the current iteration is suitable for new users: https://github.com/bitcoin-core/gui/pull/96

Sjors referenced this in commit 9df44e0b97 on Sep 18, 2020

Sjors commented at 10:41 am on September 18, 2020: member

Forgot the extended tests… fixed in #19971

Sjors referenced this in commit 165b8fd561 on Sep 18, 2020

Sjors referenced this in commit a5f5374b43 on Sep 18, 2020

fanquake referenced this in commit 967be53aee on Sep 19, 2020

sidhujag referenced this in commit 202a5e5958 on Sep 20, 2020

sidhujag referenced this in commit ecb0d694fd on Sep 20, 2020

domob1812 referenced this in commit 16219bfc2e on Sep 22, 2020

ryanofsky commented at 7:46 pm on September 29, 2020: member

re: #15454 (comment) & #15454#pullrequestreview-490569662

It seems possible to remove the additional “-wallet=” extra_arg from a number of the changed tests

#20034 follows this PR up and removes all of these. It also goes further and removes the test framework -wallet extra_args mangling, which is no longer necessary thanks to this PR and #15937

jsarenik referenced this in commit ca69ec385e on Oct 1, 2020

jsarenik referenced this in commit cd5508ebce on Oct 1, 2020

cdecker referenced this in commit 3df395ff9b on Oct 13, 2020

cdecker referenced this in commit 90acf5072d on Oct 13, 2020

ryanofsky referenced this in commit 1044521921 on Oct 19, 2020

ryanofsky referenced this in commit bc2b0d99a5 on Oct 19, 2020

ryanofsky referenced this in commit e368d2bb3b on Oct 21, 2020

ryanofsky referenced this in commit 23bc6999c2 on Oct 21, 2020

ryanofsky referenced this in commit 01476a88a6 on Oct 21, 2020

bitcoinhodler referenced this in commit 66bd08dd72 on Oct 22, 2020

MarcoFalke referenced this in commit 42b66a6b81 on Oct 29, 2020

sidhujag referenced this in commit 48063676f9 on Oct 29, 2020

MarcoFalke referenced this in commit e7986c51bc on Nov 17, 2020

sidhujag referenced this in commit 66e8029eba on Nov 17, 2020

bitcoinhodler referenced this in commit 6c0748add7 on Nov 22, 2020

vibhaa referenced this in commit 98923c7f0d on Mar 24, 2021

nixbitcoin referenced this in commit 82b3c88008 on Jul 8, 2021

nixbitcoin referenced this in commit c5884be654 on Jul 12, 2021

Fabcien referenced this in commit 407828747f on Oct 7, 2021

bitcoinfacts referenced this in commit d4f61a03b2 on Dec 27, 2021

DrahtBot locked this on Feb 15, 2022

Remove the automatic creation and loading of the default wallet #15454

Conflicts