let user select wallet file with -wallet=foo.dat

tcatm commented at 9:28 am on September 30, 2012: none

This patch lets a user have multiple wallets within his data directory by selecting a wallet using -wallet=filename. It also allows accessing wallets outside the data directory. In that case, the database environment is still stored in the data directory so both wallet file and data directory must be kept in sync (e.g. when storing the wallet within an encrypted container).

I’ve been using this patch for a few months now without any troubles and it’s proved to be pretty useful.

in src/main.cpp: in 754b81a4d6 outdated

57@@ -58,6 +58,7 @@
58 // Settings
59 int64 nTransactionFee = 0;
60 
61+const char *pszWalletFile;

Diapolo commented at 10:13 am on September 30, 2012:

I would feel better, if this would be a std::string.

laanwj commented at 10:41 am on September 30, 2012:

Me too, that would make it an owning reference to the underlying data. The memory referenced by .c_str() is deallocated when the underlying std::string goes out of scope, so after the Init() function. Keeping a global reference to it results in use-after-free problems…

gmaxwell commented at 7:39 pm on October 1, 2012: contributor

Eh. The functionality is desirable, but the ability to filesystem split the wallet and data dir is a surefire way to end up with a corrupted wallet. This is subtle and I suspect hard to warn people out of doing, esp since it would mostly work. (until it eats your keys for lunch).

BitcoinPullTester commented at 1:58 am on October 2, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/38ff7ee0239d1609a9bafd764f53fb45bbf3ac33 for binaries and test log.

sipa commented at 1:17 pm on October 7, 2012: member

I agree with @gmaxwell. Can we add a check so only wallet names without / (or other filesystem separation character) are accepted? Maybe even limit to just alphanumeric names, and add “.dat” implicitly?

gmaxwell commented at 0:27 am on October 21, 2012: contributor

Now that ultraprune has taken everything else out of the BDB environment, we can now have pluggable wallet locations, but it needs to move the whole db environment too, not just the wallet.dat location.

tcatm commented at 0:34 am on October 21, 2012: none

This patch was never meant to be used to access wallets outside $DATADIR. I’ve written it to use multiple wallets within the same datadir.

sipa commented at 0:43 am on October 21, 2012: member

Maybe not, but then it should either enforce that requirement. Alternatively, it could allow accessing a wallet file anyway, as long as the BDB dir (not the entire datadir) is moved along.

gmaxwell commented at 0:53 am on October 21, 2012: contributor

Without one of enforcement or moving the datadir it’s a serious footgun that will result in permanent coin loss. I like moving the datadir, since it’s viable now— and it would let you have your wallet file on a encrypted/removable volume while the chain resides on some less secure medium (or even a ramdisk).

BitcoinPullTester commented at 3:14 pm on October 31, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/9ccd2da8ad5a1c38db4d3bb73a8638fa6457533a for binaries and test log.

luke-jr commented at 5:56 am on November 13, 2012: member

@tcatm Any update and/or rebase?

Diapolo commented at 2:07 am on January 1, 2013: none

I guess in the light of real multiple wallet support coming along, this is not needed anymore?

laanwj commented at 7:12 pm on May 19, 2013: member

Agree with the other posts. This is very useful functionality, but this should either

Enforce the wallet.dat to be inside the data directory or
Open the database environment in the alternative path if a wallet in another path is specified. As the wallet database is the only thing that uses berkelydb now, there is no reason not to.

Personally I like (2) most.

laanwj commented at 5:19 am on June 4, 2013: member

As for (2) we really need a way to set another wallet directory (ie, walletdir=…). There is no reason anymore why the wallet directory and block chain directory would need to be the same.

luke-jr commented at 5:32 am on June 4, 2013: member

Having been using this for a few months, I have to say I prefer (1).

laanwj commented at 5:32 am on June 4, 2013: member

Why? Are you against setting the wallet directory?

luke-jr commented at 5:34 am on June 4, 2013: member

It’s easier and less disk-bloating to just use simple files, IMO.

laanwj commented at 5:36 am on June 4, 2013: member

How do you mean “just use simple files”? I don’t understand you, we aren’t changing the wallet format here. BDB will still create a database environment, and it needs to be on the same disk as the .dat file (for reasons gmaxwell mentions). So if the user wants to store the wallet on say, a removable disk, he needs the database environment there as well.

Option 1 would make this impossible (as the database environment is locked in place), and forces the user to always have the wallet in the same directory as the block chain.

luke-jr commented at 5:42 am on June 4, 2013: member

Different use cases / user preferences, I guess. Maybe if there’s a “/” on the end, interpret it as a directory, and if there isn’t, make sure there’s no “/“s at all?

laanwj commented at 5:44 am on June 4, 2013: member

Or maybe two separate options, instead of trying to parse it from one value:

0-walletdir=/dir/etc/   directory of wallet and db env (defaults to datadir)
1-wallet=bla.dat         name of wallet (defaults to wallet.dat). Within walletdir, cannot contain '/'.

luke-jr commented at 5:57 am on June 4, 2013: member

Too much shed painting; let’s just merge it as long as the main concern is solved :)

(prefer to append the file extension in code, to ease changing wallet formats later)

sipa commented at 7:21 am on June 4, 2013: member

I don’t see the problem. Just allow specifying any filename, and use database/ in the directory that file is in as a database dir? If filename doesn’t contain any /, use the datadir.

laanwj commented at 1:43 pm on June 4, 2013: member

Agree with @sipa

tcatm commented at 10:57 am on June 30, 2013: none

I’ve updated this request:

strWalletFile is now used for salvage/recover (previously those features were still using wallet.dat)
The supplied wallet file can not contain paths anymore so only plain filenames are allowed now. Even relative paths inside the data directory will be rejected.

sipa commented at 3:50 pm on June 30, 2013: member

ACK on the semantics, but can you move strWalletFile from main.cpp to init.cpp? (the validation logic has no business knowing where the wallet file is located).

tcatm commented at 7:01 pm on July 3, 2013: none

Any objections or ACKs?

sipa commented at 7:11 pm on July 3, 2013: member

ACK, if you squeeze the commits.

let user select wallet file with -wallet=foo.dat

use std::string instead of psz for WalletFile

only allow wallets within $DATADIR

Use strWalletFile in salvage/recover

fix: remove unused variable pszWalletFile

move strWalletFile to init.h/init.cpp

avoid conversion of strWalletfile to c-string

674cb304b3

sipa commented at 3:21 pm on July 4, 2013: member

ACK

BitcoinPullTester commented at 11:46 am on July 20, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/674cb304b376358fdcb17b4a0b16ae7b00cdbedc for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

gavinandresen referenced this in commit 5e67e124cf on Jul 25, 2013

gavinandresen merged this on Jul 25, 2013

gavinandresen closed this on Jul 25, 2013

tcatm deleted the branch on Jul 26, 2013

KolbyML referenced this in commit 4136d8f5a6 on Dec 5, 2020

DrahtBot locked this on Sep 8, 2021

let user select wallet file with -wallet=foo.dat #1889