doc: Add warning against wallet.dat re-use

[corollari]

Following discussion in #18205, this PR adds a warning against re-use of the same wallet file on two different nodes, as that can cause problems due to race conditions between nodes (eg: both nodes using the same addresses at the same time for different things because they are not aware of the other node).

I've also included the rationale behind the warning but I've kept it short to make it clearer to users, not sure if I should have written a longer explanation instead.

Also, while this PR may help some users avoid problems, the changes are largely inconsequential, so feel free to close it if it's not worth the effort.

On an unrelated note, I've also set up this site, which periodically pulls bitcoin core and turns its docs into a webpage. Browsing the docs can also be done locally or on github, so this doesn't add much value, but I personally find that more comfortable and it makes them more searchable.

[laanwj]

ACK 92d56e7
Not sure if this is the most visible place to document this but doing so is better than not.

[corollari]

Any thoughts on better places to put it? The reason why I put it there was because it's right next to the description of wallet.dat, which is also only bit of documentation that references it at the moment, so I hoped that people that could use this warning would naturally end up in the page where the file is described.

[luke-jr]

Should we mention doing this with JBOK wallets can also result in lost coins? (We no longer create them, but we still support them...)

[maflcko]

What is a JBOK wallet? I was running git log -S JBOK and it didn't return anything. I presume you mean pre-HD wallets?

[luke-jr]

"Just a Bunch Of Keys", yes

[ryanofsky]

Any thoughts on better places to put it?

This seems like a good place to have the documentation. But another idea might be to create readme files in newly created wallet directories (if TryCreateDirectories returns true). The readme files could say something like "This is a bitcoin wallet directory. When creating backups, please be sure to back up wallet.dat as well as any transaction log files in database/*.log to avoid corruption. Also to prevent double spends and key reuse, don't run separate bitcoin nodes with copies of the same original wallet."

[fjahr]

ACK 92d56e7

I think this is an OK location to put it, I did a quick look around and did not find a better one in a different doc. My only alternative suggestion would be to add a new section to files.md called "Sharing files". It could include hints on pitfalls when sharing files with other nodes, not just wallet.dat. I am sure there are other pitfalls/useful hint when sharing files, I just don't have any good example off the top of my head right now.

[luke-jr]

Another thing to say here: never copy the file directly, always use the backupwallet RPC or GUI menu option.

As for a "Sharing files" section: chainstate shouldn't be shared from untrusted people; blocks can be. etc

[corollari]

Another thing to say here: never copy the file directly, always use the backupwallet RPC or GUI menu option.

I've just added that in, from my understanding of the code, the only benefit that backupwallet provides over a standard copy is that it locks the wallet while the copy is being done, thus preventing any updates to it that could result in a corrupted copy, is that right?

create readme files in newly created wallet directories (if TryCreateDirectories returns true).

I quite like this idea, if other people also believe that it's better to do it that way over just adding a warning on the docs I'll change this PR to use that approach instead.

[luke-jr]

I've just added that in, from my understanding of the code, the only benefit that backupwallet provides over a standard copy is that it locks the wallet while the copy is being done, thus preventing any updates to it that could result in a corrupted copy, is that right?

No, it also "finalises" the logs into the single wallet.dat file. Without that step, copying the wallet.dat file alone can miss recent changes.

[ryanofsky]

if other people also believe that it's better to do it that way over just adding a warning on the docs I'll change this PR to use that approach instead.

To be clear, the suggestion was just supposed to about an additional place this could be documented, not an alternate place. files.md is a good place to describe the file regardless

[corollari]

No, it also "finalises" the logs into the single wallet.dat file. Without that step, copying the wallet.dat file alone can miss recent changes.

I've updated the PR to add that bit.

the suggestion was just supposed to about an additional place this could be documented

I see, I'll reserve that change for a new PR then.

My only alternative suggestion would be to add a new section to files.md called "Sharing files"

I don't know what alternative is better, i guess it depends on how often people try to copy files for other reasons. So far, the only use cases I have thought of for which you'd want to share files are:

• Wallet copy/backup
• Trusted fast-sync

For the second use case, I guess that most users that want that would prefer to get some data which is not readily available as a file currently, such as the utxo set and the last 300 blocks like btcpayserver does. Nevertheless I can see some people wanting to do a fast full-sync, so adding a warning on that might be worth it.

[laanwj]

But another idea might be to create readme files in newly created wallet directories (if TryCreateDirectories returns true).

Good idea, and to make the GUI display this.

No need to do so in thie PR though! To be clear, my point was "let's merge this we can add it in other places later" not to hold this up.

re-ACK after squash.

[maflcko]

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

[corollari]

Squashed.