WORK IN PROGRESS
Add another bitcoin-tool called bitcoin-wallet-tool. Currently it supports creation of wallets (with optional encryption before creating keys), encryption and some info-dumping.
If we agree on this concept, it could be use for creating a HD wallet with a given xpriv. Also, a such tool would probably be required to properly restore a hd wallet. If we once migrate away from BerkleyDB, this could also be helpful.
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… :/
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.
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?
-Wl,--start-group to LDFLAGS before the relevant libraries like I did here: #8873#pullrequestreview-2650430
doesn’t build for me. Fails at linking:
0libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): In function `CWallet::AddCScript(CScript const&)':
1/home/ubuntu/bitcoin/src/wallet/wallet.cpp:238: undefined reference to `CBasicKeyStore::AddCScript(CScript const&)'
2libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): In function `COutput::ToString[abi:cxx11]() const':
3/home/ubuntu/bitcoin/src/wallet/wallet.cpp:78: undefined reference to `FormatMoney[abi:cxx11](long const&)'
4libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): In function `CWallet::IsMine(CTxOut const&) const':
5/home/ubuntu/bitcoin/src/wallet/wallet.cpp:1192: undefined reference to `IsMine(CKeyStore const&, CScript const&, SigVersion)'
6...
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.
-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.