I like the concept of an external app working on wallet.dat files. I know several use cases where you have many wallet.dat files and with this, you can remove all the notes around the files you have to select between them.

I can even imagine removing/deprecating encryptwallet, walletpassphrasechange and such from bitcoind afterwards.

We have to take care of proper file locking when working on the file when bitcoind is running at the same time.

Some travis' hosts do not build (wallet/nowallet configs?).

Yeah, why not. Concept ACK. This is conceptually similar to SQL database's external utilities for repair, management, etc.
Also I suppose when there needs to be a conversion tool between old and new formats, it would be part of this utility?

From a deployment point of view I think we should keep this command-line only. Two Qt-using statically linked monster executables in the distribution is a bit too much, because of size concerns but also because e.g. MacOSX assumes a one-to-one mapping of GUI executables to applications.

We have to take care of proper file locking when working on the file when bitcoind is running at the same time.

This is indeed very important, it needs to be prevented (and tested) that it is impossible to fudge a database with this utility while bitcoind is running.

Also I suppose when there needs to be a conversion tool between old and new formats, it would be part of this utility?

Yes. The whole -walletupgare (as well as salvage) could go into this tool.

From a deployment point of view I think we should keep this command-line only. Two Qt-using statically linked monster executables in the distribution is a bit too much, because of size concerns but also because e.g. MacOSX assumes a one-to-one mapping of GUI executables to applications.

Yes. The current Qt app could support similar functions, It doesn't need to be a separate App. Multiwallet support would help in this case.

Another operation that would be useful here: rewriting the wallet. This does the CDB::rewrite step as done after encryption, but is meant to get rid of no-longer-used 'junk' blocks in the wallet. Either to make the file smaller or permanently rid of sensitive metadata that has been removed.

This cannot be conveniently done from the context of bitcoind (needs restart) but ideally in an external tool.

I think if we're going to keep adding more binaries like this, we really ought to stop using static linking for the gitian bins sooner rather than later... :/

Can't figure out how to solve the gcc linking circular dependencies issue. Maybe @theuni know a way to solve this.

I think if we're going to keep adding more binaries like this, we really ought to stop using static linking for the gitian bins sooner rather than later... :/

On windows that's easy: package the DLLs with the binary, ship them in the same directory. Many software does this. On Linux less so, unfortunately. It's possible to ship .so files but it needs a special script to invoke then, as they won't get picked up automatically if they're in the directory of the executable.

Anyhow this is a tangent: our non-GUI dependencies are really small. E.g. boost is mostly header-only with some small library files, libevent is tiny, berkeleydb is relatively small, etc. It isn't too problematic to link them to each executable. Also this tool doesn't even need libevent.

Agree with @laanwj. The non QT static linked tools should be relatively small, compared to the disk space required by the blocks/index.

It's possible to ship .so files but it needs a special script to invoke then, as they won't get picked up automatically if they're in the directory of the executable.

Hmm, what about RPATH?

Agree with @laanwj/@jonasschnelli above.

@luke-jr rpath would work (for linux, you can use a relative path). There are lots of reasons not to do that though, and unless there's a better reason than filesize, it doesn't seem justifiable.

Closing for now. Requires some steps in between until this can be revitalized

Reopening.
A such tool would make much sense for zapwallet, salvage, wallet-creation with an xpriv (especially with multiwallet).

Not sure if you are still seeing issues with circular dependencies, but a simple way to allow building with circular dependencies during development is to add -Wl,--start-group to LDFLAGS before the relevant libraries like I did here: #8873 (review)

doesn't build for me. Fails at linking:

libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): In function `CWallet::AddCScript(CScript const&)':
/home/ubuntu/bitcoin/src/wallet/wallet.cpp:238: undefined reference to `CBasicKeyStore::AddCScript(CScript const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): In function `COutput::ToString[abi:cxx11]() const':
/home/ubuntu/bitcoin/src/wallet/wallet.cpp:78: undefined reference to `FormatMoney[abi:cxx11](long const&)'
libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): In function `CWallet::IsMine(CTxOut const&) const':
/home/ubuntu/bitcoin/src/wallet/wallet.cpp:1192: undefined reference to `IsMine(CKeyStore const&, CScript const&, SigVersion)'
...

Travis seems to have the same build failures.

Concept ACK. Once you've fixed the build failures, I'm very keen to test and review this. As you say, this will be especially useful for multi-wallet.

I can get this to build using the -Wl,--start-group flag as suggested by ryanofsky above. If anyone wants to test/review this, then the branch here: https://github.com/jnewbery/bitcoin/tree/pr8745.1 builds

I have a branch of this rebased on #10762 and #12490 (which remove the wallet <-> server circular dependencies and allow this to build)

here: https://github.com/jnewbery/bitcoin/tree/pr8745_rebased if anyone wants to play around with it.

I've rebased https://github.com/jnewbery/bitcoin/tree/pr8745_rebased and pushed it (there were lots of silent conflicts with wallet refactors).

I've rebased https://github.com/jnewbery/bitcoin/tree/pr8745_rebased on top of #12490 (which removed the circular dependency that prevents this branch from building).

@jonasschnelli - feel free to take that branch and push it to this PR. Otherwise, I'm happy to open my own PR and continue maintaining this feature.

Closing in the hope @jnewbery takes this over