added `-walletallowsymboliclink` (default false)

isghe commented at 4:43 pm on September 15, 2017: contributor

When is set to 1, permits to use symbolic link as a wallet.

When is set to 1, permits to use symbolic link as a wallet.

d83c28e1a8

luke-jr commented at 5:07 pm on September 15, 2017: member

I don’t know if this is a good idea. We don’t allow wallets to be symlinks or outside the datadir for good reason (they’re not self-sufficient files).

esotericnonsense commented at 5:24 pm on September 15, 2017: contributor

#10885 contains some further information on this.

Preferred option there seems to be allowing a seperate walletdir (that would include bdb state).

wtogami commented at 5:39 pm on September 15, 2017: contributor

FWIW I used my wallet.dat as a symlink to a separate encrypted partition for years now.

gmaxwell commented at 5:50 pm on September 15, 2017: contributor

If the wallet.dat is on another filesystem from the database/ directory this will result in corruption in the event of unclean shutoffs.

d3zd3z commented at 6:01 pm on September 15, 2017: none

So, the argument is that it is better that my wallet be on an unencrypted filesystem than a chance something might get corrupt?

d3zd3z commented at 6:06 pm on September 15, 2017: none

I’m not sure I’m seeing how being on the same filesystem would prevent corruption. In my particular case, though, they are on the same filesystem, but the wallet goes through encfs to an underlying encrypted file on the filesystem.

TheBlueMatt commented at 6:08 pm on September 15, 2017: member

If you want to do this you should instead just put your datadir on your encrypted partition and symlink chainstate/ and blocks/ onto your unencrypted partition. This will leave a few (smaller) files on your encrypted partition, but that should be fine. Worst case if anything blow up the thing you lose is your blocks/chainstate, which you can re-sync, instead of your wallet. This should maybe be noted in doc/files.md.

isghe commented at 6:12 pm on September 15, 2017: contributor

I think we should suppose that, who is using symbolic links, knows the risks on the way using them.

In multi wallet configuration, the risk to load more than one time the same wallet;
The risk that the target file is in another filesystem (as @gmaxwell told before).

Anyway introducing this limitation (rejecting symbolic links) introduce a big incompatibility with previous versions, didn’t cited in release-noted.md.

For example, without symbolic links, I must change and reconsider the way I am handling my wallets, maybe using hard links, saving elsewhere, where the hard link is pointing to; that’s sounds very ugly job.

Thanks for the attentions :-)

jnewbery commented at 6:21 pm on September 15, 2017: member

I think we should suppose that, who is using symbolic links, knows the risks on the way using them.

This isn’t a problem with symbolic links per se, it’s that it’s not safe to use them for the .dat file:

bitcoind can’t distinguish whether multiple symlinks refer to the same file
it’s not safe for the .dat file to be stored on a separate file system from the database directory

Both can lead to wallet file corruption, and therefore loss of funds. I wouldn’t expect users to know this. We should prevent them from doing something that’s this dangerous.

I think a better solution is to allow the user to specify an entirely separate directory for all the wallet files.

luke-jr commented at 6:21 pm on September 15, 2017: member

Symlinked wallets have never been supported. It was a bug that we didn’t error immediately before.

Note that you won’t benefit from the encrypted filesystem wrapper in your example either: all the data will be written to the database/ directory before wallet.dat…

achow101 commented at 6:21 pm on September 15, 2017: member

So, the argument is that it is better that my wallet be on an unencrypted filesystem than a chance something might get corrupt?

No.

The wallet database creates extra temporary files in the location of where Bitcoin Core thinks your wallet is (i.e. the symlink location). If your wallet is unencrypted (i.e. not using Bitcoin Core’s wallet encryption), then those temporary files will be written to your unencrypted disk anyways and they may contain your unencrypted private keys. So keeping your wallet file on an encrypted disk is not helping as your private keys are unencrypted to the database software and may be written to an unencrypted disk while the wallet is in use.

I’m not sure I’m seeing how being on the same filesystem would prevent corruption. In my particular case, though, they are on the same filesystem, but the wallet goes through encfs to an underlying encrypted file on the filesystem.

Using symbolic linking may imply that you are attempting to use the same wallet file with different instances of Bitcoin Core (using an encrypted filesystem and symlinking is only one use case) which have different datadirs. If you were to shut down Bitcoin Core and the wallet database is not closed cleanly, there will be left over wallet database files in the datadir (i.e. not with the actual wallet file). If you then try to open it with another instance of Bitcoin Core with a different datadir, since the wallet.dat file was closed uncleanly and because the other datadir does not have the necessary wallet database files, you can have your wallet file corrupted.

esotericnonsense commented at 6:23 pm on September 15, 2017: contributor

Referencing gmaxwell’s comment and jnewbery’s above I also agree that allowing a seperate wallet directory that includes both wallet.dat and the BDB logs would be a far better solution. This folder could be specified or symlinked.

luke-jr’s comment about the database directory being unencrypted also came up on IRC in a related discussion.

I believe that the relative paths are in wallet/db.cpp. In wallet/wallet.cpp GetDataDir() could be replaced with a GetWalletDir that defaults to GetDataDir but can be overridden.

I don’t have a requirement for this particular feature but it doesn’t look like too much of a pain to change.

Polve commented at 6:41 pm on September 15, 2017: none

If you want to do this you should instead just put your datadir on your encrypted partition and symlink chainstate/ and blocks/ onto your unencrypted partition

How this would solve the corruption problems in the event of unclean shutoffs gmax is talking about?

achow101 commented at 6:48 pm on September 15, 2017: member

How this would solve the corruption problems in the event of unclean shutoffs gmax is talking about?

All of the relevant database files needed for the wallet.dat file would be present in the datadir on the encrypted disk instead of separated on different disks. They would actually be found in the same folder together as they are supposed to be.

Polve commented at 7:05 pm on September 15, 2017: none

All of the relevant database files needed for the wallet.dat file would be present in the datadir on the encrypted disk instead of separated on different disks

wouldn’t this be the same case where only the .dat file is symlinked to another folder/partition? (that’s my usual use case, like @wtogami, on an encrypted partition)

achow101 commented at 7:13 pm on September 15, 2017: member

wouldn’t this be the same case where only the .dat file is symlinked to another folder/partition? (that’s my usual use case, like @wtogami, on an encrypted partition)

No it is not. When only the wallet file is symlinked, the wallet database files are created in the Bitcoin Core datadir which contains the symlink. They are not created in the folder containing the actual file itself. When you point the datadir and symlink the chainstate and blocks folder, the wallet database files will be in the same folder as the wallet file itself.

Here’s some poor ascii art:

What happens when you symlink wallet.dat

0Datadir on unencrypted disk | Folder on encrypted disk 
1--------------------------- | -----------------------
2 wallet database temp files |
3 symlink to wallet.dat file | -------> actual wallet.dat file 
4              blocks folder |
5          chainstate folder |

What happens when you symlink other datadir stuff

0Folder on unencrypted disk | Datadir on encrypted disk 
1-------------------------- | -----------------------
2                           | wallet database temp files 
3                           | actual wallet.dat file
4      actual blocks folder | <----- symlink to blocks folder 
5  actual chainstate folder | <----- symlink chainstate folder

isghe commented at 8:20 pm on September 15, 2017: contributor

My point of view is that who is using symbolic links, knows what it is doing:

I know that:

I am not using multi wallet;
If I will use multi wallet, I’ll take care not passing the same files to bitcoin-core;
the original file is not on another file system;
I am not generating dynamically new addresses;
I can lose the original wallet.dat (that’s why I make regular backups);

Supposing that using symbolic links, will not affect the global Bitcoin, but eventually only my local system, why should bitcoin-core take care, about the good or wrong way, I am using symbolic links? A warning message shouldn’t be enough?

Thanks :-)

jnewbery commented at 8:42 pm on September 15, 2017: member

The fact that it is safe for you to use in no way implies that it’s safe in general to use. Contributors to this project consider the needs of all users, not just those who are as sophisticated and knowledgeable as you.

isghe commented at 8:56 pm on September 15, 2017: contributor

@jnewbery didn’t you consider that the possible workarounds, a user can adopt, to go over this limitation (rejecting symbolic links), would be more dangerous, than giving to the user the choice to use it or not at his own risk?

jnewbery commented at 9:05 pm on September 15, 2017: member

didn’t you consider…

Yes, and my personal judgement is that taking previously undocumented behaviour which could result in loss of funds and turning it into a documented option is irresponsible.

isghe commented at 9:28 pm on September 15, 2017: contributor

@jnewbery interesting point of view, I’ll think on it. Thanks :-)

achow101 commented at 9:47 pm on September 15, 2017: member

My point of view is that who is using symbolic links, knows what it is doing:

But do you really? Do you know the other BDB temporary files created are not in the same directory as the actual wallet file when you use symbolic links? Those files are actually in your datadir with the symlink, not the actual wallet file. Do you know that by using symbolic links you are at more risk of corrupting your wallet and losing your money? Do you know that keeping an unencrypted wallet on an encrypted drive and then symlinking it is completely pointless since your unencrypted keys will probably be written to the unencrypted drive anyways due to the BDB temp files?

There are a lot of things that most people are not going to know or consider, even if they know what they are doing with symbolic links. Just because you are technical enough to use symlinks does not mean that you fully understand how Bitcoin Core will work with this symlinks and any consequences of that.

isghe commented at 9:52 pm on September 15, 2017: contributor

@achow101 I think I wrote very well I am not symlinking to another file system.

achow101 commented at 10:01 pm on September 15, 2017: member

I brought up the file systems thing for others who were talking about encrypted file systems. The other points still stand as they don’t depend on other file systems.

fanquake added the label Wallet on Sep 16, 2017

laanwj commented at 7:59 am on September 16, 2017: member

NACK. My preferred approach would be: Wallets should be in their own directory #11348

isghe commented at 11:48 pm on September 20, 2017: contributor

I don’t understand why the limitation no wallet.dat as symbolic link introduced in v.0.15, is not declared in https://github.com/bitcoin/bitcoin/blob/v0.15.0/doc/release-notes.md Thanks

sipa commented at 0:02 am on September 21, 2017: member

@isghe It’s not in the release notes because it was not supported functionality, and I don’t think anyone was aware of users relying on this.

In any case, NACK. This is a footgun. I prefer #11348.

MarcoFalke commented at 9:03 am on September 22, 2017: member

This pull points to the wrong branch (we only merge on the master branch). Also, there is disagreement whether this is the right direction.

MarcoFalke closed this on Sep 22, 2017

sylvandb commented at 9:54 pm on November 15, 2017: none

Symbolic links have nothing to do with specific applications and have been system supported functionality for decades. Bitcoin core intentionally preventing that functionality is EGOCENTRIC, RUDE and as it wasn’t documented in the release notes, INCONSIDERATE.

Every reason cited above in the threads on this topic justifies nothing more than a warning which the user should be free to bypass at their own risk.

You have totally destroyed years of my wallet management without even a mention in the release notes and left me wondering what other bombs you have left me to discover.

Thanks so very much.

MarcoFalke commented at 10:04 pm on November 15, 2017: member

If you are happy to accept loss of funds, you are very welcome to remove the fs::is_symlink(wallet_path) check in init.cpp. After all, this is open source. 🎉

ryanofsky commented at 10:07 pm on November 15, 2017: member

@sylvandb, we have some changes queued up which should allow more flexibility. I have #11687 and there is also #11466. Right this second I am working on an improvement to #11687 that will actually reallow symlinks and address lukejr’s concerns in that PR.

ryanofsky commented at 10:09 pm on November 15, 2017: member

By the way, I was also surprised that there didn’t seem to be a mention of symlink change in the release notes. That does seem like an unfortunate oversight. I wish more prs would update documentation and code together.

sylvandb commented at 10:15 pm on November 15, 2017: none

@MarcoFalke already done. How do you propose to address my other concern about potential for “other bombs” left in the code or the loss of trust caused by hidden, incompatible changes? @ryanofsky that’s good news.

All, usually I try to leave a constructive idea with a gripe. Sorry for the first post where I forgot to do that, so here it is:

The correct approach to resolve symlink problems and protect users does require to detect them and handle appropriately. In this case I suspect that would be to de-reference to the real path and use that instead of the path containing the symlink. However because core writes a bunch of stuff with canned filenames to accompany the wallet file, those names also need to change because the symlink may be selecting one file from a directory containing multiple wallet files.

MarcoFalke commented at 10:19 pm on November 15, 2017: member

@sylvandb I (and others) are paying close attention to every pull and commit to see if it might break some use case or interface. We will then mention that change not only in the list of changes in the release notes, but also devote a section or short sentence to it. However, there is only a limited amount of people that care about the release notes and oversights happen from time to time. This is an example of such oversight, sadly.

MarcoFalke commented at 10:24 pm on November 15, 2017: member

Also, when it comes to the wallet it is hard to reason about the different ways people use the wallet. This makes it tedious to review new wallet features, which often ends in rejection of wallet features and limiting the usability. Rather err on the save side, than letting inexperienced users lose funds.

isghe commented at 11:50 pm on November 16, 2017: contributor

I merged this patch -walletallowsymboliclink to version v0.15.1 in branch https://github.com/isghe/bitcoin/tree/isghe_0.15.1.

For my purposes it just works. Anyway take care the risks using symbolic link, not only with bitcoin, but also with a text editor…

Thanks, Isidoro

luke-jr commented at 11:55 pm on November 16, 2017: member

There’s no comparison. Text files are single standalone files.

isghe commented at 0:16 am on November 17, 2017: contributor

I don’t know any other software inhibiting symbolic links. Why is it necessary in bitcoin, and why it is not described in release note?

sipa commented at 0:23 am on November 17, 2017: member

Because Bitcoin Core (BDB) creates temporary database journal files in the same directory as the wallet files. If those database files end up on a different filesystem, and a crash happens, there is no more guarantee the file is in sync with the journals. When people move files around (which often happens) between directories, it’s even harder to reason about consistency.

You’re free to use any patch you like, but the conditions under which it’s safe to do so are too complicated to explain in release notes. We should just explain it is not supported. And it not being mentioned in release notes was indeed an oversight - we should fix that.

isghe commented at 0:37 am on November 17, 2017: contributor

so, please add in release note, something similar to: “for security reasons we don’t allow anymore symbolic links to wallet.dat files”

luke-jr commented at 0:51 am on November 17, 2017: member

It was never supported, and never safe.

sipa commented at 0:52 am on November 17, 2017: member

@luke-jr I agree with that, but still it makes sense to communicate that this situation is now detected.

sylvandb commented at 7:56 pm on November 17, 2017: none

@luke-jr of course it was supported. Not explicitly, but implicitly, likely the same way core supports running on an SSD versus a mechanical hard disk or possibly even an AMD vs. Intel processor.

There are a lot of relative safety arguments for SSD vs. mechanical, and there are unique failure cases for a symlink vs. real file in the directory.

In my case, a symlink is safer because it allows me to keep my wallet files together where they all have daily encrypted backup with a history going back several years. Changing a symlink to change wallets is a lot less error prone and risky than copying a wallet file into the active location and copying it back out some time later.

So now my choice is to run my own patched version of core, or take increased risk with my wallet files.

Polve commented at 6:33 am on November 18, 2017: none

My (forced) choice was to not upgrade.

meshcollider commented at 7:50 am on November 18, 2017: contributor

@Polve as mentioned above, hopefully the next major release (0.16.0) should support having wallet files outside the data directory (see #11466 which is basically ready to merge) in a way which is much safer than symlinking, apologies for the inconvenience caused until then :)

DrahtBot locked this on Sep 8, 2021

added -walletallowsymboliclink (default false) #11343

added `-walletallowsymboliclink` (default false) #11343