do not truncate .dat extension for wallets in gui #12795

Truncating the extension results in wallet name ambiguity and the inability to use the wallet in GUI debug rpc console.

Resolves #12794

utACK fc7c32fc68f2c2618644f9bea6176c63940706b3

utACK fc7c32fc68f2c2618644f9bea6176c63940706b3

Tested ACK fc7c32f.

Running bitcoin-qt -regtest -wallet=w1 -wallet=w2 -wallet=/tmp/foo.dat -wallet=/tmp/foo:

• RPC listwallets: <img width="310" alt="screen shot 2018-03-26 at 21 17 36" src="https://user-images.githubusercontent.com/3534524/37930615-2c2f58b4-313b-11e8-90ec-5e589ae243d8.png">

• Console selector: <img width="288" alt="screen shot 2018-03-26 at 21 17 28" src="https://user-images.githubusercontent.com/3534524/37930621-32055fae-313b-11e8-9a44-c8ddd8793653.png">

• Wallet view selector: <img width="272" alt="screen shot 2018-03-26 at 21 17 13" src="https://user-images.githubusercontent.com/3534524/37930632-3a1e63de-313b-11e8-84d5-836a73f75c23.png">

@promag could you also check you can do wallet calls with *.dat properly?

utACK fc7c32fc68f2c2618644f9bea6176c63940706b3

utACK fc7c32fc68f2c2618644f9bea6176c63940706b3

@instagibbs switching between foo.dat and foo and calling getbalance yields different results, so works as expected.

This change improves the quality of the software because it presents information to the user more accurately.

@MarcoFalke 2nd time this particular job timed out: https://travis-ci.org/bitcoin/bitcoin/jobs/358567658

@instagibbs Its fine to ignore, I guess.

Tested ACK https://github.com/bitcoin/bitcoin/pull/12795/commits/fc7c32fc68f2c2618644f9bea6176c63940706b3

NACK, users should not be exposed to file extensions...

@luke-jr they already are, they're just not exposed to .dat...

If we simply disallow files with names that clash, maybe?

users should not be exposed to file extensions

Why? Is that a some sort of UI best practice? Specially after #11687 I fail to see an alternative.

What do you mean they already are?

I don't see any reason to disallow names that clash - but it should be harmless as well since that would be immensely confusing to users anyway.

Why? Is that a some sort of UI best practice? Specially after #11687 I fail to see an alternative.

The alternative is to use the wallet selected, despite visual appearance (which shouldn't matter).

(Note that the original multiwallet PR didn't have this bug; it was introduced with regressions in jonas's branch that somehow got merged despite me pointing out out over and over.)

@luke-jr current behavior in master allows blah.wallet, blah.dat, blah, side by side. Which is why I suggested disallowing collisions as a different fix.

@instagibbs They should all show simply "blah", and work correctly if the user bothers to figure out which is which.

NACK, users should not be exposed to file extensions...

Should be little risk of this, because since #11687 new wallets are created as directories instead of files. The only way a user would be shown a .dat extension (or any berkeleydb data filename) is if they explicitly configured bitcoin to load a .dat file rather than a wallet directory.

@luke-jr I'm sorry that's far more unfriendly; users shouldn't be confronted with two identical labels

@instagibbs Ordinarily, they won't be, because naming two wallets the same thing is dumb. @ryanofsky That only affects new wallets AFAIK?

@ryanofsky That only affects new wallets AFAIK?

That's right, new wallets are created as directories with separate berkeleydb environments, but for backwards compatibility it's still possible to load individual .dat files in a shared environment.

In general I agree that it's bad to expose file extensions in the UI, and thankfully going forward this will not happen in either the gui or regular daemon because of the switch to wallet directories.

In the backwards compatible case it seems more counterintuitive to drop ".dat" suffixes, than to simply use filenames as provided by the user without transforming them. Not transforming filenames has the advantage that wallets can be identified with the same names everywhere: on the command line, in conf files, in listwallets and getwalletinfo output, and in the gui. This means fewer opportunities for confusion.

It would be really ugly if the software were adding extensions automatically and then leaking them into the gui, exposing internal layout of wallet databases. But that's not what is happening here. What's happening here is that the user is specifying a filename with an extension and the software is automatically removing it, making it more difficult than it needs to be to identify and distinguish wallets. Better, IMO, to just use names exactly as provided by the user.

utACK fc7c32fc68f2c2618644f9bea6176c63940706b3 Travis issue seems unrelated.

From the GUI perspective, users don't want to deal with filename. They want to create wallets and give them a name and eventually rename them later as well as use all sorts of special characters.

In the long run, the GUI should allow to dynamically create wallets where the user given wallet name could be completely different then the filename (filename could be a uuid similar to how other GUI applications work [like calendar apps]).

I think it makes sense to show the full file name for older wallets, to prevent confusion.

It would be nice if there was a way to "upgrade" them.

If we do want to make legacy wallet file names look pretty, I would suggest stripping the file extension, then checking for duplicates and showing the full file name in those cases.

Adding name metadata to the wallet payload makes sense too, although decades of word processors have accustomed people to file names matching the window title when they open a document.