Remind developers that disablewallet=1 should always work #13677

A user may configure disablewallet=1 for instance to prevent a wallet from accidentally being created (which could lead to confusion as to what to backup).

Since #13059 a user might also use that option to prevent someone with RPC access from dynamically loading a wallet there're not supposed to.

The most rigorous way to do the above is to compile with --disable-wallet.

As of #13112 unknown config arguments throw an error. So the user in the above scenario would have to remove this line. But then later if they download a new binary with wallet support compiled, they might forget to add it back. For that reason I believe disablewallet=1 should always be allowed.

Although unknown config arguments throw an error, the wallet interface added in #10762 includes a dummy wallet to handle --disable-wallet. Part of this dummy wallets adds disablewallet and other wallet parameters to the list of hidden params; those aren't shown in help, but their presence won't throw an error. This might change in the some future refactor though

So I added a comment that hopefully prevents a future refactor from causing disablewallet=1 to throw when compiled with --disable-wallet.

AFAIU the test https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_disable.py would fail if there was such refactor. However this test is not executed because:

https://github.com/bitcoin/bitcoin/blob/a2dbe2a108add34a532b0b8df44dd27bf59b10cd/test/functional/test_runner.py#L240-L243

WDYT about improving this?

Otherwise utACK 262bdcb.

Perhaps we can make compiling with --disable-wallet imply: (a) default for GetArg("-disablewallet") becomes true (b) throw an error when -disablewallet=0 is specified

I forgot that Travis runs the functional test suite with --disable-wallet as well: https://github.com/bitcoin/bitcoin/blob/master/.travis.yml#L40

Will look into @promag & @sipa's suggestions, which should make this more robust than merely a comment.

<!--5d09a71f8925f3f132321140b44b946d-->The last travis run for this pull request was 53 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

I forgot that Travis runs the functional test suite with --disable-wallet as well: https://github.com/bitcoin/bitcoin/blob/master/.travis.yml#L40

This is not (yet) true. See https://github.com/bitcoin/bitcoin/pull/14180

I think this can be closed?

@MarcoFalke #14180 covers progag's suggestion, but I still need to implement sipa's suggestion.

Please let me know when you want to continue working on this, so the pull request can be re-opened.