084e17cebd424b8e8ced674bc810eef4e6ee5d3b Remove unused includes (practicalswift)
Pull request description:
As requested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/16273#issuecomment-521332089:
This PR removes unused includes.
Please note that in contrast to #16273 I'm limiting the scope to the trivial cases of pure removals (i.e. no includes added) to make reviewing easier.
I'm seeking "Concept ACK":s for this obviously non-urgent minor cleanup.
Rationale:
* Avoids unnecessary re-compiles in case of header changes.
* Makes reasoning about code dependencies easier.
* Reduces compile-time memory usage.
* Reduces compilation time.
* Warm fuzzy feeling of being lean :-)
ACKs for top commit:
ryanofsky:
Code review ACK 084e17cebd424b8e8ced674bc810eef4e6ee5d3b. PR only removes include lines and it still compiles. In the worst case someone might have to explicitly add an include later for something now included implicitly. But maybe some effort was taken to avoid this, and it wouldn't be a tragedy anyway.
Tree-SHA512: 89de56edc6ceea4696e9579bccff10c80080821685b9fb4e8c5ef593b6e43cf662f358788701bb09f84867693f66b2e4db035b92b522a0a775f50b7ecffd6a6d
4a86a0acd9ac3ca392f0584a5fd079a856e5e4ba Make SafeDbt DB_DBT_MALLOC on default initialization (Ben Woosley)
1a9f9f7e5e2e73fb832f5b96ad7e9e57954f3f3c Introduce SafeDbt to handle DB_DBT_MALLOC raii-style (Ben Woosley)
951a44e9cd6cf2b8058244f3f95181c5ba683fdd Drop unused setRange arg to BerkeleyBatch::ReadAtCursor (Ben Woosley)
Pull request description:
This provides additional exception-safety and case handling for the proper
freeing of the associated buffers.
Tree-SHA512: a038d728290cdb3905e7d881608052a6675b6425729ceaf7cfe69a6e91c2ee293cdb01e4b695a20963459ffdd9d4a1f9a08b3c07b1b5ba1aa8590a8149f686db
faf369880 wallet: Improve log output for errors during load (Glenn Willen)
Pull request description:
When loading the wallet, display the entire path in error messages, instead of
the name (which, for the default wallet, is the empty string.)
When an exception occurs during wallet loading, display e.what() if possible,
instead of nothing.
Tree-SHA512: 435247628db669579bb694ba4b53ba174fe42c0329fc72f09fc274bb28463ee69f53412abb2a3b45bb8f59f7eb928c0167e397b8d0a514135142192a87294614
14bc2a17dd03ccd89f65a302328763ff22c710c2 Trivial: add doxygen-compatible comments relating to BerkeleyEnvironment (Pierre Rochard)
88b1d956fe3e38f2d2dd805feee9dadb0be9e8a9 Tests: add unit tests for GetWalletEnv (Pierre Rochard)
f1f4bb7345b90853ec5037478173601035593d26 Free BerkeleyEnvironment instances when not in use (Russell Yanofsky)
Pull request description:
Instead of adding BerkeleyEnvironment objects permanently to the g_dbenvs map, use reference counted shared pointers and remove map entries when the last BerkeleyEnvironment reference goes out of scope.
This change was requested by @TheBlueMatt and makes code that sets up mock databases cleaner. The mock database environment will now go out of scope and be reset on destruction so there is no need to call BerkeleyEnvironment::Reset() during wallet construction to clear out prior state.
This change does affect bitcoin behavior slightly. On startup, instead of same wallet environments staying open throughout VerifyWallets() and OpenWallets() calls, VerifyWallets() will open and close an environment once for each wallet, and OpenWallets() will create its own environment(s) later.
Tree-SHA512: 219d77a9e2268298435b86088f998795e059fdab1d2050ba284a9ab8d8a44961c9b5cf96e94ee521688108d23c6db680e3e3a999b8cb2ac2a8590f691d50668b
591203149f1700f594f781862e88cbbfe83d8d37 wallet: Create IsDatabaseLoaded function (Chun Kuan Lee)
15c93f075a881deb3ad7b1dd8a4516a9b06e5e11 wallet: Add trailing wallet.dat when detecting duplicate wallet if it's a directory. (Chun Kuan Lee)
c456fbd8dfcc748e5ec9feaa57ec0f2900f99cde Refactor: Move m_db pointers into BerkeleyDatabase (Russell Yanofsky)
Pull request description:
Fix#14538
Fix crash attempting to load the same wallet with different path strings that resolve to the same absolute path. The primary check which prevents loading the same wallet twice is:
6b8d0a2164/src/wallet/db.cpp (L44)
But this check is skipped if both wallet paths resolve to the same absolute path, due to caching here:
6b8d0a2164/src/wallet/db.cpp (L467)
Meanwhile a secondary check for duplicate wallets is not reliable because it based on a literal comparison, instead of comparison using absolute paths:
6b8d0a2164/src/wallet/wallet.cpp (L3853)
This PR fixes the latter check to compare the absolute path of a new wallet being loaded to absolute paths of wallets already loaded, so there should no longer be any way to load the same wallet more than once.
Tree-SHA512: 2fa01811c160b57be3b76c6b4983556a04bbce71a3f8202429987ec020664a062e897deedcd9248bc04e9baaa2fc7b464e2595dcaeff2af0818387bf1fcdbf6f
4ea77320c5f0b275876be41ff530bb328ba0cb87 tests: add test case for loading copied wallet twice (Chun Kuan Lee)
2d796faf62095e83f74337c26e7e1a8c3957cf3c wallet: Fix duplicate fileid (Chun Kuan Lee)
Pull request description:
The implementation in current master can not detect if the file ID is duplicate with flushed `BerkeleyEnvironment`. This PR would store the file ID in a global variable `g_fileids` and release it when the `BerkeleyDatabase` close. So it won't have to rely on a `Db*`.
Fix#14304
Tree-SHA512: 0632254b696bb4c671b5e2e5781e9012df54ba3c6ab0f919d9f6d31f374d3b0f8bd968b90b537884ac8c3d2906afdd58c2ce258666263464c7dbd636960b0e8f
c1dde3a949b36ce9c2155777b3fa1372e7ed97d8 No longer shutdown after encrypting the wallet (Andrew Chow)
d7637c5a3f1d62922594cdfb6272e30dacf60ce9 After encrypting the wallet, reload the database environment (Andrew Chow)
5d296ac810755dc47f105eb95b52b7e2bcb8aea8 Add function to close all Db's and reload the databae environment (Andrew Chow)
a769461d5e37ddcb771ae836254fdc69177a28c4 Move BerkeleyEnvironment deletion from internal method to callsite (Andrew Chow)
Pull request description:
This is the replacement for #11678 which implements @ryanofsky's [suggestion](https://github.com/bitcoin/bitcoin/pull/11678#pullrequestreview-76464511).
Shutting down the software was to prevent the BDB environment from writing unencrypted private keys to disk in the database log files, as was noted [here](https://bitcointalk.org/index.php?topic=51474.msg616068#msg616068). This PR replaces the shutdown behavior with a CDBEnv flush, close, and reopen which achieves the same effect: everything is cleanly flushed and closed, the log files are removed, and then the environment reopened to continue normal operation.
To ensure that no unencrypted private keys are in the log files after encrypting the wallet, I wrote [this script](https://gist.github.com/achow101/7f7143e6c3d3fdc034d3470e72823e9d) to pull private keys from the original wallet file and searches for these keys in the log files (note that you will have to change your file paths to make it work on your own machine).
As for concerns about private keys being written to slack space or being kept in memory, these behaviors no longer exist after the original wallet encryption PR and the shutting down solution from 2011.
cc @ryanofsky
Tree-SHA512: 34b894283b0677a873d06dee46dff8424dec85a2973009ac9b84bcf3d22d05f227c494168c395219d9aee3178e420cf70d4b3eeacc9785aa86b6015d25758e75
be8ab7d08 Create new wallet databases as directories rather than files (Russell Yanofsky)
26c06f24e Allow wallet files not in -walletdir directory (Russell Yanofsky)
d8a99f65e Allow wallet files in multiple directories (Russell Yanofsky)
Pull request description:
This change consists of three commits:
* The first commit is a pure refactoring that removes the restriction that two wallets can only be opened at the same time if they are contained in the same directory.
* The second commit removes the restriction that `-wallet` filenames can only refer to files in the `-walletdir` directory.
* The third commit makes second commit a little safer by changing bitcoin to create wallet databases as directories rather than files, so they can be safely backed up.
All three commits should be straightforward:
* The first commit adds around 20 lines of new code and then updates a bunch of function signatures (generally updating them to take plain fs::path parameters, instead of combinations of strings, fs::paths, and objects like CDBEnv and CWalletDBWrapper).
* The second commit removes two `-wallet` filename checks and adds some test cases to the multiwallet unit test.
* The third commit just changes the mapping from specified wallet paths to bdb environment & data paths.
---
**Note:** For anybody looking at this PR for the first time, I think you can skip the comments before _20 Nov_ and start reading at https://github.com/bitcoin/bitcoin/pull/11687#issuecomment-345625565. Comments before _20 Nov_ were about an earlier version of the PR that didn't include the third commit, and then confusion from not seeing the first commit.
Tree-SHA512: 00bbb120fe0df847cf57014f75f1f7f1f58b0b62fa0b3adab4560163ebdfe06ccdfff33b4231693f03c5dc23601cb41954a07bcea9a4919c8d42f7d62bcf6024
2f3bd47 Abstract directory locking into util.cpp (MeshCollider)
5260a4a Make .walletlock distinct from .lock (MeshCollider)
64226de Generalise walletdir lock error message for correctness (MeshCollider)
c9ed4bd Add a test for wallet directory locking (MeshCollider)
e60cb99 Add a lock to the wallet directory (MeshCollider)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/11888, needs a 0.16 milestone
Also adds a test that the lock works.
https://github.com/bitcoin/bitcoin/pull/11687 will probably rework this to a per-wallet lock instead of just the walletdir, but this fixes the current issue
Tree-SHA512: 98e52d67f820e3b8f919cf361ffbb7d928f1bd67603e0ed26c5076ea02d9b3a90c3535ddf7329f3b88171396fa28dd3c87adab3577a8a217bd1e4247bda99138
* scripted-diff: Replace #include "" with #include <> (ryanofsky)
-BEGIN VERIFY SCRIPT-
for f in \
src/*.cpp \
src/*.h \
src/bench/*.cpp \
src/bench/*.h \
src/compat/*.cpp \
src/compat/*.h \
src/consensus/*.cpp \
src/consensus/*.h \
src/crypto/*.cpp \
src/crypto/*.h \
src/crypto/ctaes/*.h \
src/policy/*.cpp \
src/policy/*.h \
src/primitives/*.cpp \
src/primitives/*.h \
src/qt/*.cpp \
src/qt/*.h \
src/qt/test/*.cpp \
src/qt/test/*.h \
src/rpc/*.cpp \
src/rpc/*.h \
src/script/*.cpp \
src/script/*.h \
src/support/*.cpp \
src/support/*.h \
src/support/allocators/*.h \
src/test/*.cpp \
src/test/*.h \
src/wallet/*.cpp \
src/wallet/*.h \
src/wallet/test/*.cpp \
src/wallet/test/*.h \
src/zmq/*.cpp \
src/zmq/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Signed-off-by: Pasta <pasta@dashboost.org>
* scripted-diff: Replace #include "" with #include <> (Dash Specific)
-BEGIN VERIFY SCRIPT-
for f in \
src/bls/*.cpp \
src/bls/*.h \
src/evo/*.cpp \
src/evo/*.h \
src/governance/*.cpp \
src/governance/*.h \
src/llmq/*.cpp \
src/llmq/*.h \
src/masternode/*.cpp \
src/masternode/*.h \
src/privatesend/*.cpp \
src/privatesend/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Signed-off-by: Pasta <pasta@dashboost.org>
* build: Remove -I for everything but project root
Remove -I from build system for everything but the project root,
and built-in dependencies.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/Makefile.test.include
* qt: refactor: Use absolute include paths in .ui files
* qt: refactor: Changes to make include paths absolute
This makes all include paths in the GUI absolute.
Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/qt/dash.cpp
# src/qt/optionsmodel.cpp
# src/qt/test/rpcnestedtests.cpp
* test: refactor: Use absolute include paths for test data files
* Recommend #include<> syntax in developer notes
* refactor: Include obj/build.h instead of build.h
* END BACKPORT #11651 Remove trailing whitespace causing travis failure
* fix backport 11651
Signed-off-by: Pasta <pasta@dashboost.org>
* More of 11651
* fix blockchain.cpp
Signed-off-by: pasta <pasta@dashboost.org>
* Add missing "qt/" in includes
* Add missing "test/" in includes
* Fix trailing whitespaces
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
a357293 Use MakeUnique<Db>(...) (practicalswift)
3e09b39 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (practicalswift)
8617989 Add MakeUnique (substitute for C++14 std::make_unique) (practicalswift)
d223bc9 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (practicalswift)
b45c597 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift)
29ab96d Use unique_ptr for dbenv (DbEnv) (practicalswift)
f72cbf9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift)
8ccf1bb Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift)
73db063 Use unique_ptr for upnp_thread (boost::thread) (practicalswift)
0024531 Use unique_ptr for dbw (CDBWrapper) (practicalswift)
fa6d122 Use unique_ptr:s for {fee,short,long}Stats (TxConfirmStats) (practicalswift)
5a6f768 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift)
860e912 Use unique_ptr for pwalletMain (CWallet) (practicalswift)
Pull request description:
Use `std::unique_ptr` (C++11) where possible.
Rationale:
1. Avoid resource leaks (specifically: forgetting to `delete` an object created using `new`)
2. Avoid undefined behaviour (specifically: double `delete`:s)
**Note to reviewers:** Please let me know if I've missed any obvious `std::unique_ptr` candidates. Hopefully this PR should cover all the trivial cases.
Tree-SHA512: 9fbeb47b800ab8ff4e0be9f2a22ab63c23d5c613a0c6716d9183db8d22ddbbce592fb8384a8b7874bf7375c8161efb13ca2197ad6f24b75967148037f0f7b20c
c1e5d40 Make debugging test crash easier (MeshCollider)
8263f6a Create walletdir if datadir doesn't exist and fix tests (MeshCollider)
9587a9c Default walletdir is wallets/ if it exists (MeshCollider)
d987889 Add release notes for -walletdir and wallets/ dir (MeshCollider)
80c5cbc Add test for -walletdir (MeshCollider)
0530ba0 Add -walletdir parameter to specify custom wallet dir (MeshCollider)
Pull request description:
Closes#11348
Adds a `-walletdir` parameter which specifies a directory to use for wallets, allowing them to be stored separately from the 'main' data directory. Creates a new `wallets/` directory in datadir if this is the first time running, and defaults to using it if it exists.
Includes tests and release notes. Things which might need to be considered more:
- there is no 'lock' on the wallets directory, which might be needed?
- because this uses a new wallets/ directory by default, downgrading to an earlier version won't see the wallets in that directory (not a big deal though, users can just copy them up to the main dir)
- jnewbery suggested putting each wallet in its own directory, which is a good idea, but out of scope for this PR IMO. EDIT: this is being done in https://github.com/bitcoin/bitcoin/pull/11687
- doc/files.md needs updating (will do soon)
I also considered including a cleanup by removing caching of data directory paths and instead just initialise them once on startup (c.f. #3073), but decided it wasn't super relevant here will just complicate review.
Tree-SHA512: c8ac04bfe9a810c32055f2c8b8fa0d535e56125ceb8d96f12447dd3538bf3e5ee992b60b1cd2173bf5f3fa023a9feab12c9963593bf27ed419df929bb413398d
2a07f878a Refactor: Modernize disallowed copy constructors/assignment (Dan Raviv)
Pull request description:
Use C++11's better capability of expressing an interface of a non-copyable class by publicly deleting its copy ctor and assignment operator instead of just declaring them private.
Tree-SHA512: 878f446be5a136bb2a90643aaeaca62948b575e6ef71ccc5b4b8f373e66f36ced00665128f36504e0ccfee639863d969329c4276154ef9f2a9de9137f0801e01
* Merge #8694: Basic multiwallet support
c237bd7 wallet: Update formatting (Luke Dashjr)
9cbe8c8 wallet: Forbid -salvagewallet, -zapwallettxes, and -upgradewallet with multiple wallets (Luke Dashjr)
a2a5f3f wallet: Base backup filenames on original wallet filename (Luke Dashjr)
b823a4c wallet: Include actual backup filename in recovery warning message (Luke Dashjr)
84dcb45 Bugfix: wallet: Fix warningStr, errorStr argument order (Luke Dashjr)
008c360 Wallet: Move multiwallet sanity checks to CWallet::Verify, and do other checks on all wallets (Luke Dashjr)
0f08575 Wallet: Support loading multiple wallets if -wallet used more than once (Luke Dashjr)
b124cf0 Wallet: Replace pwalletMain with a vector of wallet pointers (Luke Dashjr)
19b3648 CWalletDB: Store the update counter per wallet (Luke Dashjr)
74e8738 Bugfix: ForceSetArg should replace entr(ies) in mapMultiArgs, not append (Luke Dashjr)
23fb9ad wallet: Move nAccountingEntryNumber from static/global to CWallet (Luke Dashjr)
9d15d55 Bugfix: wallet: Increment "update counter" when modifying account stuff (Luke Dashjr)
f28eb80 Bugfix: wallet: Increment "update counter" only after actually making the applicable db changes to avoid potential races (Luke Dashjr)
Tree-SHA512: 23f5dda58477307bc07997010740f1dc729164cdddefd2f9a2c9c7a877111eb1516d3e2ad4f9b104621f0b7f17369c69fcef13d28b85cb6c01d35f09a8845f23
* pwalletMain -> vpwallets
Signed-off-by: Pasta <pasta@dashboost.org>
* add gArgs
Signed-off-by: Pasta <pasta@dashboost.org>
* continued pwalletsMain -> vpwallets
Signed-off-by: Pasta <pasta@dashboost.org>
* remove external pwalletMains and pwalletMain -> vpwallet
Signed-off-by: Pasta <pasta@dashboost.org>
* add external referance to vpwallets
Signed-off-by: Pasta <pasta@dashboost.org>
* more pWalletMain -> vpwallets[0]
Signed-off-by: Pasta <pasta@dashboost.org>
* code review
Signed-off-by: Pasta <pasta@dashboost.org>
* revert LOCK(cs_main) move
Signed-off-by: Pasta <pasta@dashboost.org>
* import wallet.h and remove extern
Signed-off-by: Pasta <pasta@dashboost.org>
* batch.* -> WriteIC and EraseIC
Signed-off-by: Pasta <pasta@dashboost.org>
* wrap wallet.h include inside of an ifdef
Signed-off-by: Pasta <pasta@dashboost.org>
* drop wallet.h
Signed-off-by: Pasta <pasta@dashboost.org>
* add dropped "!"
Signed-off-by: Pasta <pasta@dashboost.org>
8e69adc66 Add missing include for atomic in db.h (Alex Morcos)
Tree-SHA512: 1fb9a8b3eb6238f3ee754d94cf1424fd51bbb2ec6780dcb50fdc3f190fd6c2e8567b5f01b25c4197b86ee684eddcf345d24723220154cf595e3b96ad24e611ac
b51aaf1 Remove unused C++ code not covered by unit tests (practicalswift)
Tree-SHA512: 267bbd87df01a296bf23e82a8b6ee968e13e23a6aaecc535d803890a3e3e9f6208c7fc4c1f97afd98ed3e498b12fe1ada7e3cb2977ad12359a813f57336c74e5
911a480 wallet: Add comment describing the various classes in walletdb.h (Wladimir J. van der Laan)
69d2e9b wallet: Make IsDummy private in CWalletDBWrapper (Wladimir J. van der Laan)
3323281 wallet: CWalletDB CDB composition not inheritance (Wladimir J. van der Laan)
be9e1a9 wallet: Reduce references to global bitdb environment (Wladimir J. van der Laan)
071c955 wallet: Get rid of fFileBacked (Wladimir J. van der Laan)
71afe3c wallet: Introduce database handle wrapper (Wladimir J. van der Laan)
Tree-SHA512: e4e72953c61a2f6995d609a32f8ed8e18cab9a92bc9e193d46a1d1f06d9daa5c6da6fce2867d4e3ba4fc0439141901a3d35f246486f0fa8f59587786379dfcbd
add 'batch.'
Signed-off-by: Pasta <Pasta@dash.org>
remove `if (fFileBacked)`
Signed-off-by: Pasta <pasta@dashboost.org>
remove fFileBacked and strWalletFile
Signed-off-by: Pasta <Pasta@dash.org>
f110272 Remove `namespace fs=fs` (Wladimir J. van der Laan)
75594bd torcontrol: Use fs::path instead of std::string for private key path (Wladimir J. van der Laan)
2a5f574 Use fsbridge for fopen and freopen (Wladimir J. van der Laan)
bac5c9c Replace uses of boost::filesystem with fs (Wladimir J. van der Laan)
7d5172d Replace includes of boost/filesystem.h with fs.h (Wladimir J. van der Laan)
19e36bb Add fs.cpp/h (Wladimir J. van der Laan)
Tree-SHA512: 2c34f059dfa6850b9323f3389e9090a6b5f839a457a2960d182c2ecfafd9883c956f5928bb796613402d3aad68ebc78259796a7a313f4a6cfa98aaf507a66842
5113474 wallet: Use CDataStream.data() (Wladimir J. van der Laan)
e2300ff bench: Use CDataStream.data() (Wladimir J. van der Laan)
adff950 dbwrapper: Use new .data() method of CDataStream (Wladimir J. van der Laan)
a2141e4 streams: Remove special cases for ancient MSVC (Wladimir J. van der Laan)
af4c44c streams: Add data() method to CDataStream (Wladimir J. van der Laan)
- fixes#3136
- the problem is related to Boost path and a static initialized internal
pointer
- using a std::string in CDBEnv::EnvShutdown() prevents the problem
- this removes the boost::filesystem::path path field from CDBEnv
