Move all user data to home directory

[AdamISZ]

To facilitate easier management by users and to
follow generally accepted standards, this PR moves
the following all to user home directory, subdir
.JoinMarket :
joinmarket.cfg file
wallets/ directory
logs/ directory
cmtdata/ directory
commitmentlist file

An info message is added on startup showing location.

[AdamISZ]

Note:
The testing setup will be borked by this; it may take some meaningful work to fix it up, I will investigate shortly.

[AdamISZ]

This was discussed tangentially previously in #62 and #420 and is identified as a goal in TODO.md ... we should have done this a long time ago imo but it is a little tricky, we have to be careful not to miss some corner case.

[chris-belcher]

It would be useful to have a command line argument --homedir which allows a user-configured location. Also maybe then testing can be fixed by adding --homedir . which puts the data in the same location as before.

[kristapsk]

Maybe just a preference and not important, but why .JoinMarket instead of .joinmarket?

[AdamISZ]

Maybe just a preference and not important, but why .JoinMarket instead of .joinmarket?

Yeah I think lower case is better. Will change.

It would be useful to have a command line argument --homedir which allows a user-configured location. Also maybe then testing can be fixed by adding --homedir . which puts the data in the same location as before.

Makes sense. Will do.

[AdamISZ]

Pushed a big change with a lot of edits; mainly this comes out of wanting to add the homedir option, then realising it relates back to #430 i.e. I don't want to add a bunch of repeats. So added add_base_options() to include recoversync, wallet-password-stdin and homedir options to all scripts - note, I considered it's only necessary to make sure function of an option doesn't conflict, it doesn't matter if it's unused.
Then realised that to make the refactor work, needed to move cli_options.py module out of scripts/ and into jmclient package (hence a bunch of changes to imports). It's debatable where it better belongs, because it isn't a user script so better not to have it there, and some functionality is useful within jmclient/ but it's arguable and I don't think it matters too much.

So this addresses the homedir issue (including for tests as well) - the default is ~/.joinmarket (no longer JoinMarket), but a user can specify --homedir for any script and change that - but also addresses, I would say 80% fulfils, #430 .. as I was saying to @kristapsk in some other thread, it's kind of not desirable to ignore that and go ahead and add more repeats all over the codebase, so better just fold it in if it's a necessary part of the job, which it is here.

[AdamISZ]

Passing tests locally so seems there is some travis misbehaviour (see https://travis-ci.org/JoinMarket-Org/joinmarket-clientserver/jobs/628110243#L16377). Not that it really matters of course. This still requires quite a bit of testing and review.

[AdamISZ]

Oh - mustn't forget to add changes to docs along with this PR.

[bitcoinland]

So this addresses the homedir issue (including for tests as well) - the default is ~/.joinmarket (no longer JoinMarket), but a user can specify --homedir for any script and change that - but also addresses, I would say 80% fulfils, #430 .. as I was saying to @kristapsk in some other thread, it's kind of not desirable to ignore that and go ahead and add more repeats all over the codebase, so better just fold it in if it's a necessary part of the job, which it is here.

Sorry for jumping in, long time lurker, first time commenting, but since it defaults to ~/.joinmarket and not "~/" wouldn't it make more sense to call the parameter datadir instead of homedir?

Thanks and congrats to everybody on the great tool.

[AdamISZ]

Sorry for jumping in, long time lurker, first time commenting, but since it defaults to ~/.joinmarket and not "~/" wouldn't it make more sense to call the parameter datadir instead of homedir?

Yes. I was thinking that in an idle moment, except I was thinking perhaps appdir or appdatadir ... to be fair, it includes configuration as well as logs and wallets .. not entirely clear what you call that.
I almost think userdir is clearer, even if it's not the same idea.

Does everyone prefer datadir out of the various possibilities?

[kristapsk]

Passing tests locally so seems there is some travis misbehaviour (see https://travis-ci.org/JoinMarket-Org/joinmarket-clientserver/jobs/628110243#L16377).

They also fail on my computer the same way.

[kristapsk]

They fail because of missing joinmarket.cfg. Copying ./test/regtest_joinmarket.cfg to ~/.joinmarket/joinmarket.cfg is current workaround. All tests should use --homedir option instead of default homedir.

[AdamISZ]

I see, thanks for spotting that. I misinterpreted the error message in travis and, as you say, inadvertently tested with a valid regtest joinmarket.cfg in the homedir ( or datadir!).

Will fix shortly with some kind of startup setting of jm_single().homedir in the test setup phase.

[AdamISZ]

So 8e0d05b as per the commit comment solves the test config location problem with a wrapper function; instead of calling load_program_config() all tests will now call load_test_config which will default to the local dir instead of ~/.joinmarket.
There were a few options here but none of them avoided making some trivial change (like this one) to all the test files, or otherwise creating awkwardness. This seems good enough; there are now changes to every test file, but at least that change is absolutely trivial in every case (function name change only).

A small point but the location of the function load_test_config within jmclient/configure was kind of the only logical one since that's where all the tests are currently pulling load_program_config from (including tests outside the packages in /tests.

[AdamISZ]

Will do some testing from here, I think the code is done.
Edit: except I forgot, I will change homedir to datadir everywhere if everyone agrees.

[kristapsk]

Spent some time looking at how other wallets handle this. I think it's important to think carefully about the end result as this is something that will be P.I.T.A. to change again later.

First thing is the directory structure itself. A lot of wallets separate mainnet, testnet and regtest with a subdirectories. It makes sense as you will not use mainnet wallet on a testnet and vice versa. And probably having logs, etc... separate also is good idea.

Here's some existing examples of the structure (not complete, only stuff that's relevant):

1. Bitcoin Core

.bitcoin/
+- regtest/
|  +- (regtest wallets and other stuff)
+- testnet3/
|  +- (testnet wallets and other stuff)
+- bitcoin.conf
+- (mainnet wallets and other stuff)

2. Electrum

.electrum/
+- testnet/
|  +- config
|  +- wallets/
|      +- (testnet wallets)
+- config
+- wallets/
|  +- (mainnet wallets)

3. Wasabi Wallet

It looks like it acts the same way as JM currently, you can actually load mainnet wallet in a testnet, no separate subdirectories, etc...

I personally would like for JM something like Bitcoin Core does with global config file, but either all other stuff for non-mainnet going into separate subdirectories or even mainnet having separate subdirectory for everything except joinmarket.cfg.

About homedir vs datadir - datadir is probably better, Bitcoin Core also calls it that way.

[kristapsk]

Noticed two issues with the GUI:

• "Wallet->Load" browse dialog defaults to scripts/wallets instead of new datadir wallet directory;
• When generating new wallet, it is generated into old scripts/wallets directory instead of new datadir wallet directory and you get "Failed to open wallet ...: not a file." error (the same likely happens with wallet-tool too, didn't test that).

[AdamISZ]

About homedir vs datadir - datadir is probably better, Bitcoin Core also calls it that way.

Yes there seems little disagreement on it. Will change.

I personally would like for JM something like Bitcoin Core does with global config file, but either all other stuff for non-mainnet going into separate subdirectories or even mainnet having separate subdirectory for everything except joinmarket.cfg.

I did think about this a little when doing the update to the testing suite. I considered one option being a regtest/ subdirectory (influenced by what bitcoin does, as you say) but I think I didn't do it to avoid a bunch of extra work updating tests.

An interesting question is whether it's actually OK to not use different directories. Wallet .jmdat files encode their network type within them, so there's no danger of confusion there. For commitment related data (commitmentlist and stuff in cmtdata/ and a few special files (example: TUMBLE.schedule, perhaps others), however, I think it would be cleaner to have completely separate directories, even if I don't think anything disastrous is going to happen without that (maybe with external commitments? I'd have to check; also don't think anyone is using them!).

Noticed two issues with the GUI:

Thanks for getting on with testing already :) I'll address those two points.

[AdamISZ]

Rebased on master.

Addressed the errors in the Qt wallet load as pointed out by @kristapsk ; changed the name from homedir to datadir; updated the usage guide documentation.

Any more tests would be welcome. Basically any scripts or Qt functions.

[AdamISZ]

So to expand on the previous comment about whether using one directory is OK, and whether commitments data is an exception to that rule:

1. First a quick refresher on how the commitments data works: There are two different data structures kept persisted. First, the simple one is the commitmentlist file, currently kept in the joinmarket-clientserver root directory. This is used by Makers as a blacklist (if the commitment is already in the list, a new transaction using it is not allowed). Second, there is a dict of form {'used': [], 'external':{}}. This is stored by default in joinmarket-clientserver/cmtdata/commitments.json and now moved to ~/.joinmarket/cmtdata/commitments.json (again by default, assuming a custom datadir is not set). This is used by Takers to keep track of what commitments they have used, and what they have decided to make available for doing coinjoins if they choose, using other utxos not in the wallet. This last feature ("external commitments") is either rarely used, or not at all; I've never heard anyone ask about it, in fact. It can be set using the add-utxo.py script.
2. The commitmentlist file won't be affected by being shared between testnet and mainnet. It's a very simple one-shot look up of a 32 byte value every time a join is requested, and so has no performance issues unless ludicrously large (which doesn't happen). So I see no need to change this. However! - there is a quirk here. This file is part of jmdaemon not jmclient (because it's just public hex values sent over the network) and therefore doesn't have access to config. For this reason I've decided that for now I'll keep it in the root directory of the source, not move it to datadir. Note it's public, broadcast data, so zero sensitivity. Reworking to put it into user directory is an unnecessary annoyance at this point.
3. The commitments.json is a bit different, because of the external commitments mentioned above. Since they are sourced from in cases where it isn't possible to find a utxo in-wallet with commitments available, an error could be caused if there was a mix of testnet and mainnet here. So I've chosen the simplest and least code-invasive solution to avoid that: on startup, if non-mainnet is detected, the location is hardcoded to <user datadir>/cmtdata/testnet_commitments.json instead of the default commitments.json. This will allow conflict between testnet and regtest, but that doesn't matter.

I believe the above justifies the one very small change as in 71b8ed4 and that from here we can happily just use one directory instead of several subdirectories. A reminder: wallets define the network they apply to when created, they cannot be changed after, so no possible confusion there.

I have tested transactions on regtest and mainnet and sanity checked that this functions as intended (including using add-utxo.py).

[kristapsk]

Isn't it /Users/username/Library/Application support/joinmarket/ on macOS, so kinda different and worth mentioning? But somebody more experienced with mac's could comment here.

[AdamISZ]

Yes, I didn't mention it because I wasn't exactly sure the right string; "similarly" is sufficiently vague to cover that :) I'll leave it like this until someone confirms the exactly right version.

[AdamISZ]

Have been using this live for a while now. Intend to merge it shortly if no objections.

[kristapsk]

I will do some testing of this today or tomorrow, as I haven't after my last comment of Qt GUI issues (which should be fixed now).

[kristapsk]

Did some more tests and things look good with wallet-tool.py and joinmarket-qt.py (although that hanged up for me once with no meaningful console output after opening wallet, but not sure that's related), but yield generators now want full path as a wallet file argument instead of just wallet filename as before.

[AdamISZ]

but yield generators now want full path as a wallet file argument instead of just wallet filename as before

oh? not for me, but i am using default location for wallets. Are you seeing that problem with --datadir? I will take a look into it.

[AdamISZ]

@kristapsk i can't replicate that problem, I must have missed a certain way to do it?

When I do python yg-privacyenhanced.py --datadir=testdir mywallet.jmdat it works, when I created ./testdir and ./testdir/joinmarket.cfg and put the wallet in ./testdir/wallets/mywallet.jmdat.

Same if I use --datadir=. ... in fact it also works fine if I specify a relative directory that doesn't exist, it just creates it and then populates it with a new config and the necessary directories, as expected.
Could you be more specific exactly what you're seeing?

[kristapsk]

@AdamISZ Couldn't it be that you have wallet files both in ~/.joinmarket/wallets/ and ./wallets/ relative to yg script (forgot to remove / rename old directory)?

[AdamISZ]

Yes, I thought of that and deleted it temporarily and tried again, it made no difference.

[kristapsk]

This is output for me:

(jmvenv) bitcoin@zibens:~/git/joinmarket-clientserver/scripts $ python wallet-tool.py wallet.jmdat summary
User data location: /home/bitcoin/.joinmarket/
Enter wallet decryption passphrase: 
2020-01-05 22:15:00,814 [INFO]  Detected new wallet, performing initial import
JM wallet
Balance for mixdepth 0: 0.00000000
Balance for mixdepth 1: 0.00000000
Balance for mixdepth 2: 0.00000000
Balance for mixdepth 3: 0.00000000
Balance for mixdepth 4: 0.00000000
Total balance:  0.00000000
(jmvenv) bitcoin@zibens:~/git/joinmarket-clientserver/scripts $ python yg-privacyenhanced.py wallet.jmdat
User data location: /home/bitcoin/.joinmarket/
Traceback (most recent call last):
  File "yg-privacyenhanced.py", line 98, in <module>
    minsize=minsize, gaplimit=gaplimit)
  File "/home/bitcoin/git/joinmarket-clientserver/jmclient/jmclient/yieldgenerator.py", line 248, in ygmain
    gap_limit=options.gaplimit)
  File "/home/bitcoin/git/joinmarket-clientserver/jmclient/jmclient/wallet_utils.py", line 1130, in open_test_wallet_maybe
    return open_wallet(path, mixdepth=max_mixdepth, **kwargs)
  File "/home/bitcoin/git/joinmarket-clientserver/jmclient/jmclient/wallet_utils.py", line 1150, in open_wallet
    raise Exception("Failed to open wallet at '{}': not a file".format(path))
Exception: Failed to open wallet at 'wallets/wallet.jmdat': not a file
(jmvenv) bitcoin@zibens:~/git/joinmarket-clientserver/scripts $ python yg-privacyenhanced.py ~/.joinmarket/wallets/wallet.jmdat
User data location: /home/bitcoin/.joinmarket/
Enter wallet decryption passphrase: 
2020-01-05 22:15:55,970 [INFO]  Detected new wallet, performing initial import
2020-01-05 22:15:56,670 [ERROR]  You do not have the minimum required amount of coins to be a maker: 1000000
2020-01-05 22:15:56,671 [INFO]  Failed to create offers, giving up.
...

[AdamISZ]

Thanks, all clear now.
So this is nothing to do with --datadir but it was a bug you found in the yieldgenerator wallet loading code.
I didn't see it because the same wallet existed in local wallets/ directory (should have been more careful, oh well). Very trivial fix, will push now.

[kristapsk]

Did some more testing with 47f6200, didn't notice any new bugs, apart from the functionality I didn't test before, it's "Wallet > Export keys" in GUI. Resulting joinmarket-private-keys.json file is saved in a scripts directory, but maybe also should go to datadir? But probably needs to be changed that user can choose file name and destination anyway.

[AdamISZ]

Thanks. I always forget to test the privkey export, yes I agree, in datadir for now and later make it choosable. Will commit this but then squash and merge.