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
fdc3293 Document assumptions that are being made to avoid NULL pointer dereferences (practicalswift)
Pull request description:
Document assumptions (via `assert(…)`:s) that are being made avoid `NULL` pointer dereferences.
Rationale:
* Make it clear to human reviewers and non-human static analyzers that what might look like potential `NULL` pointer dereferences are written the way they are intentionally (these cases are currently flagged by various static analyzers).
Tree-SHA512: b424328195e2680e1e4ec546298f718c49e5ad182147dc004de580693db1b50eec4065e1c4f232bdb302baa12954265a50ba21cb5ba4ff30248535b2de778672
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.
713a92073 Remove usehd option and warn when it is used (Andrew Chow)
d4c18f733 Bump wallet version number to 159900 (Andrew Chow)
Pull request description:
Bump the wallet version number to 159900 so that new wallets made without a default key will no longer work on previous versions at all. Also remove the `usehd` option to avoid weird interaction with wallet version numbers and HD-ness of wallets.
Tree-SHA512: dd7965505bfad6a926c79afd423236f509229a398a8398076f8d57d90a5974243f9459a61225c4daee560c796f427445c9e55a3ad528a3a97a9123ca6a1269ab
5d2a3995e [trivial] fixup comment for VerifyWallets() (John Newbery)
43b0e81d0 [wallet] Add StartWallets() function to wallet/init.cpp (John Newbery)
290f3c56d [wallet] Add RegisterWalletRPC() function to wallet/init.cpp (John Newbery)
062d63102 [wallet] Add CloseWallets() function to wallet/init.cpp (John Newbery)
77fe07c15 [wallet] Add StopWallets() function to wallet/init.cpp (John Newbery)
2da5eafa4 [wallet] Add FlushWallets() function to wallet/init.cpp (John Newbery)
1b9cee66e [wallet] Rename WalletVerify() to VerifyWallets() (John Newbery)
9c76ba18c [wallet] Rename InitLoadWallet() to OpenWallets() (John Newbery)
Pull request description:
Apologies for the mostly code move only PR. This is a pre-req for both #10740 and #10762
All wallet component initialization/destruction functions are now in their own `wallet/init.cpp` translation unit and are no longer static functions on the CWallet class. The bitcoin_server also no longer has any knowledge that there are multiple wallets in vpwallet.
There should be no changes in behavior from this PR.
Tree-SHA512: 7c260eb094f2fa1a88d803769ba60935810968a7309f731135e4b17623b97f18c03bbcd293c942093d1efce62c6c978f9ff484d54dc9a60bc2fcb5af2d160fcd
Rationale:
- this init function can now open multiple wallets (hence
Wallet->Wallets)
- This is named as the antonym to CloseWallets(), which carries out the
opposite action.
592404f03 Changing &vec[0] to vec.data(), what 9804 missed (MeshCollider)
Pull request description:
This just continues the work of https://github.com/bitcoin/bitcoin/pull/9804
Modifies a lot of `&vector[]`'s to `vector.data()`'s across all the files including tests, just the stuff that 9804 missed
Tree-SHA512: dd1a9dffb999dea4fba78dcc91fe02f90250db86f5c74948e1ff3e8b4036b2154b600555eaa04dece5368920aae3513bc36425dc96e4319ca1041b0928a6b656
fe09b0197 add missing lock to crypter GetKeys() (Marko Bencun)
5cb3da04b keystore GetKeys(): return result instead of writing to reference (Marko Bencun)
Pull request description:
Issue: #10905
First commit makes GetKeys() return the result instead of writing to a reference to remove some useless lines.
Tree-SHA512: bb51255b5a6cf5488c3d5dee89f539d41f0717f018441d120047f877e0a705a133fb3b7a97d1cf8f73b5d2ed93dd2dbdfcd6f394e40105af2a12e01d397cb402
864cd2787 Move CBitcoinAddress to base58.cpp (Pieter Wuille)
5c8ff0d44 Introduce wrappers around CBitcoinAddress (Pieter Wuille)
Pull request description:
This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`.
As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions.
This change is far from complete. In follow-ups I'd like to:
* Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so.
* Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere).
* Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`.
However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction.
Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e
This patch removes the need for the intermediary Base58 type
CBitcoinAddress, by providing {Encode,Decode,IsValid}Destination
function that directly operate on the conversion between strings
and CTxDestination.
47ba2c312 Fix currency/fee-rate unit string in the help text (Akio Nakamura)
Pull request description:
1. The RPC help text should use the constant `CURRENCY_UNIT` defined in `policy/feerate.cpp` instead of the literal `'BTC'`.
In the following 2 RPC commands, `'BTC'` is written directly in the help text.
This commit changes them to use that constant.
1) `estimatesmartfee`
2) `estimaterawfee`
2. Some RPC command use `'satoshis'` as the unit.
It should be written as `'satoshis'` instead of `'Satoshis'` in the RPC help text.
So, this commit fixes this typo in `getblocktemplate`.
Tree-SHA512: d0bd1cd90560e59bf456b076b958a2a1c998f85a7e65aeb6b2abcaba18919a3ae62f7c3909210461084c1a3275a35b6ba3ea3ec8f5cce33702ffe383c9e84bce
1. The RPC help text should use the constant CURRENCY_UNIT defined in
policy/feerate.cpp instead of the literal 'BTC'. In the following
2 RPC commands, 'BTC' is written directly in the help text.
1) estimatesmartfee
2) estimaterawfee
And also, for these help strings, the notation
'fee-per-kilobyte (in BTC)' is somewhat ambiguous.
To write more precisely, this commit changes to 'fee rate in BTC/kB'
with using the constant CURRENCY_UNIT.
2. Some RPC command use 'satoshis' as the unit. It should be written
as 'satoshis' instead of 'Satoshis' in the RPC help text.
So, this commit fixes this typo in getblocktemplate.
3. The phrase that '... feerate (BTC per KB) ...' is used to explain
the fee rate in the help text of following 2 RPC commands.
1) getmempoolinfo
2) fundrawtransaction
But they are different from other similar help text of the RPCs.
And also, 'KB' implies Kibibyte (2^10 byte).
To unify and to clarify, this commit changes these phrase to
'... fee rate in BTC/kB ...'.
(BTC references the constant 'CURRENCY_UNIT')
This contains most of the changes of 10563 "remove safe mode", but doesn't
remove the safe mode yet, but put an `ObserveSafeMode()` check in
individual calls with okSafeMode=false.
This cleans up the ugly "okSafeMode" flag from the dispatch tables,
which is not a concern for the RPC server.
Extra-author: Wladimir J. van der Laan <laanwj@gmail.com>
de9a1db Acquire cs_main lock before cs_wallet during wallet initialization (Russell Yanofsky)
Pull request description:
`CWallet::MarkConflicted` may acquire the `cs_main` lock after `CWalletDB::LoadWallet` acquires the `cs_wallet` lock during wallet initialization. (`CWalletDB::LoadWallet` calls `ReadKeyValue` which calls `CWallet::LoadToWallet` which calls `CWallet::MarkConflicted`). This is the opposite order that `cs_main` and `cs_wallet` locks are acquired in the rest of the code, and so leads to `POTENTIAL DEADLOCK DETECTED` errors if bitcoin is built with `-DDEBUG_LOCKORDER`.
This commit changes `CWallet::LoadWallet` (which calls `CWalletDB::LoadWallet`) to acquire both locks in the standard order.
Error was reported by @luke-jr in https://botbot.me/freenode/bitcoin-core-dev/msg/90244330/
Tree-SHA512: 353fe21bc0a4a2828b41876897001a3c414d4b115ee7430925bd391d8bc396fca81661145d00996c1ba1a01516d9acf8b89fb5c3da27092f5f3aa7e37ef26ffa
6af49dd Output a bit more information for fee calculation report. (Alex Morcos)
a54c7b9 Fix rounding errors in calculation of minimum change size (Alex Morcos)
Pull request description:
Thanks to @juscamarena for reporting this.
Please backport to 0.15.
There was a potential rounding error where the fee for the change added to the fee for the original tx could be less than the fee for the tx including change.
This is fixed in the first commit. The second commit adds one more snippet of information in the fee calculation report. I actually realized that there is more information that would be nice to report, but we can add that post 0.15.
An open question is whether we should be returning failure if the test in line 2885 is hit or just resetting pick_new_inputs and continuing. Originally I made it a failure to avoid any possible infinite loops. But the case hit here is an example of where that logic possibly backfired.
Tree-SHA512: efe049781acc1f6a8ad429a689359ac6f7b7c44cdfc9578a866dff4a2f6596e8de474a89d25c704f31ef4f8c89af770e98b75ef06c25419d5a6dfc87247bf274
f01103c MOVEONLY: Init functions wallet/wallet.cpp -> wallet/init.cpp (Russell Yanofsky)
e7fe320 MOVEONLY: Fee functions wallet/wallet.cpp -> wallet/fees.cpp (Russell Yanofsky)
d97fe20 Move some static functions out of wallet.h/cpp (Russell Yanofsky)
Pull request description:
This just moves some static wallet fee and init functions out of `wallet/wallet.cpp` and into new `wallet/fees.cpp` and `wallet/init.cpp` source files. There is one commit updating declarations and callers, followed by two MOVEONLY commits actually moving the function bodies.
This change is desirable because wallet.h/cpp are monolithic and hard to navigate, so pulling things out and grouping together pieces of related functionality should improve the organization.
Another motivation is the wallet process separation work in https://github.com/bitcoin/bitcoin/pull/10973, where (at least initially) parameter parsing and fee estimation are still done in the main process rather than the wallet process, and having functions that run in different processes scrambled up throughout wallet.cpp is unnecessarily confusing.
Tree-SHA512: 6e6982ff82b2ab4e681c043907e2b1801ceb9513394730070f16c46ad338278a863f5b3759aa13db76a259b268b1c919c81f4e339f0796a3cfb990161e8c316d
CWallet::MarkConflicted may acquire the cs_main lock after
CWalletDB::LoadWallet acquires the cs_wallet lock during wallet initialization.
(CWalletDB::LoadWallet calls ReadKeyValue which calls CWallet::LoadToWallet
which calls CWallet::MarkConflicted). This is the opposite order that cs_main
and cs_wallet locks are acquired in the rest of the code, and so leads to
POTENTIAL DEADLOCK DETECTED errors if bitcoin is built with -DDEBUG_LOCKORDER.
This commit changes CWallet::LoadWallet (which calls CWalletDB::LoadWallet) to
acquire both locks in the standard order. It also fixes some tests that were
acquiring wallet and main locks out of order and failed with the new locking in
CWallet::LoadWallet.
Error was reported by Luke Dashjr <luke-jr@utopios.org> in
https://botbot.me/freenode/bitcoin-core-dev/msg/90244330/
a897d0e tests: Remove OldSetKeyFromPassphrase/OldEncrypt/OldDecrypt (practicalswift)
Pull request description:
Reduces the number of non-free:d allocs with four (Δ in use at exit = -928 bytes).
With this patch applied:
```
$ valgrind --leak-check=full --show-leak-kinds=all src/test/test_bitcoin --log_level=test_suite --run_test=wallet_crypto
…
==20243== HEAP SUMMARY:
==20243== in use at exit: 72,704 bytes in 1 blocks
==20243== total heap usage: 53,138 allocs, 53,137 frees, 49,600,420 bytes allocated
==20243==
==20243== 72,704 bytes in 1 blocks are still reachable in loss record 1 of 1
==20243== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20243== by 0x6AA5EFF: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==20243== by 0x40106B9: call_init.part.0 (dl-init.c:72)
==20243== by 0x40107CA: call_init (dl-init.c:30)
==20243== by 0x40107CA: _dl_init (dl-init.c:120)
==20243== by 0x4000C69: ??? (in /lib/x86_64-linux-gnu/ld-2.23.so)
==20243== by 0x2: ???
==20243== by 0xFFF0006A2: ???
==20243== by 0xFFF0006B8: ???
==20243== by 0xFFF0006CF: ???
==20243==
==20243== LEAK SUMMARY:
==20243== definitely lost: 0 bytes in 0 blocks
==20243== indirectly lost: 0 bytes in 0 blocks
==20243== possibly lost: 0 bytes in 0 blocks
==20243== still reachable: 72,704 bytes in 1 blocks
==20243== suppressed: 0 bytes in 0 blocks
```
Without this patch applied:
```
$ valgrind --leak-check=full --show-leak-kinds=all src/test/test_bitcoin --log_level=test_suite --run_test=wallet_crypto
…
==19023== HEAP SUMMARY:
==19023== in use at exit: 73,632 bytes in 5 blocks
==19023== total heap usage: 52,718 allocs, 52,713 frees, 49,502,962 bytes allocated
==19023==
==19023== 24 bytes in 1 blocks are still reachable in loss record 1 of 5
==19023== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19023== by 0x642DE77: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E5665: lh_insert (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E7BB3: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E87AD: ERR_get_state (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E883D: ERR_put_error (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64EAAE4: EVP_DecryptFinal_ex (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x3AD150: wallet_crypto::OldDecrypt(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, secure_allocator<unsigned char> >&, unsigned char const*, unsigned char const*) (crypto_tests.cpp:81)
==19023== by 0x3AF892: wallet_crypto::TestCrypter::TestDecrypt(CCrypter const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&) (crypto_tests.cpp:137)
==19023== by 0x3AD5E9: wallet_crypto::decrypt::test_method() (crypto_tests.cpp:223)
==19023== by 0x3ADC11: wallet_crypto::decrypt_invoker() (crypto_tests.cpp:216)
==19023== by 0x182596: invoke<void (*)()> (callback.hpp:56)
==19023== by 0x182596: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
==19023==
==19023== 128 bytes in 1 blocks are still reachable in loss record 2 of 5
==19023== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19023== by 0x642DE77: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E5331: lh_new (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E7862: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E7B7F: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E87AD: ERR_get_state (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E883D: ERR_put_error (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64EAAE4: EVP_DecryptFinal_ex (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x3AD150: wallet_crypto::OldDecrypt(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, secure_allocator<unsigned char> >&, unsigned char const*, unsigned char const*) (crypto_tests.cpp:81)
==19023== by 0x3AF892: wallet_crypto::TestCrypter::TestDecrypt(CCrypter const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&) (crypto_tests.cpp:137)
==19023== by 0x3AD5E9: wallet_crypto::decrypt::test_method() (crypto_tests.cpp:223)
==19023== by 0x3ADC11: wallet_crypto::decrypt_invoker() (crypto_tests.cpp:216)
==19023==
==19023== 176 bytes in 1 blocks are still reachable in loss record 3 of 5
==19023== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19023== by 0x642DE77: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E530F: lh_new (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E7862: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E7B7F: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E87AD: ERR_get_state (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E883D: ERR_put_error (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64EAAE4: EVP_DecryptFinal_ex (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x3AD150: wallet_crypto::OldDecrypt(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, secure_allocator<unsigned char> >&, unsigned char const*, unsigned char const*) (crypto_tests.cpp:81)
==19023== by 0x3AF892: wallet_crypto::TestCrypter::TestDecrypt(CCrypter const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&) (crypto_tests.cpp:137)
==19023== by 0x3AD5E9: wallet_crypto::decrypt::test_method() (crypto_tests.cpp:223)
==19023== by 0x3ADC11: wallet_crypto::decrypt_invoker() (crypto_tests.cpp:216)
==19023==
==19023== 600 bytes in 1 blocks are still reachable in loss record 4 of 5
==19023== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19023== by 0x642DE77: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E8745: ERR_get_state (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64E883D: ERR_put_error (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x64EAAE4: EVP_DecryptFinal_ex (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==19023== by 0x3AD150: wallet_crypto::OldDecrypt(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, secure_allocator<unsigned char> >&, unsigned char const*, unsigned char const*) (crypto_tests.cpp:81)
==19023== by 0x3AF892: wallet_crypto::TestCrypter::TestDecrypt(CCrypter const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&) (crypto_tests.cpp:137)
==19023== by 0x3AD5E9: wallet_crypto::decrypt::test_method() (crypto_tests.cpp:223)
==19023== by 0x3ADC11: wallet_crypto::decrypt_invoker() (crypto_tests.cpp:216)
==19023== by 0x182596: invoke<void (*)()> (callback.hpp:56)
==19023== by 0x182596: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
==19023== by 0x596CCB0: ??? (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==19023== by 0x594C995: boost::execution_monitor::catch_signals(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==19023==
==19023== 72,704 bytes in 1 blocks are still reachable in loss record 5 of 5
==19023== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19023== by 0x6AA5EFF: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==19023== by 0x40106B9: call_init.part.0 (dl-init.c:72)
==19023== by 0x40107CA: call_init (dl-init.c:30)
==19023== by 0x40107CA: _dl_init (dl-init.c:120)
==19023== by 0x4000C69: ??? (in /lib/x86_64-linux-gnu/ld-2.23.so)
==19023== by 0x2: ???
==19023== by 0xFFF0006A2: ???
==19023== by 0xFFF0006B8: ???
==19023== by 0xFFF0006CF: ???
==19023==
==19023== LEAK SUMMARY:
==19023== definitely lost: 0 bytes in 0 blocks
==19023== indirectly lost: 0 bytes in 0 blocks
==19023== possibly lost: 0 bytes in 0 blocks
==19023== still reachable: 73,632 bytes in 5 blocks
==19023== suppressed: 0 bytes in 0 blocks
==19023==
==19023== For counts of detected and suppressed errors, rerun with: -v
==19023== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
```
Tree-SHA512: 38b6552736a5710a42dbad770c490583cfc762acbec716f5db4cf38314f494ea99430713ea407c73b49d867676ced221a282437f3fcfd8346f8f68386f4fc74d
745d2e3 Clean up getbalance RPC parameter handling (Russell Yanofsky)
fd5d71e Update developer notes after params.size() cleanup (Russell Yanofsky)
e067673 Avoid treating null RPC arguments different from missing arguments (Russell Yanofsky)
e666efc Get rid of redundant RPC params.size() checks (Russell Yanofsky)
Pull request description:
This is a followup to #10783.
- The first commit doesn't change behavior at all, just simplifies code.
- The second commit just changes RPC methods to treat null arguments the same as missing arguments instead of throwing type errors.
- The third commit updates developer notes after the cleanup.
- The forth commit does some additional code cleanup in `getbalance`.
Followup changes that should happen in future PRs:
- [ ] Replace uses of `.isTrue()` with calls to `.get_bool()` so numbers, objects, and strings cause type errors instead of being interpreted as false. https://github.com/bitcoin/bitcoin/pull/11050#discussion_r133850525
- [ ] Add braces around if statements. https://github.com/bitcoin/bitcoin/pull/11050#discussion_r133851133
- [ ] Maybe improve UniValue type error exceptions and eliminate RPCTypeCheck and RPCTypeCheckArgument functions. https://github.com/bitcoin/bitcoin/pull/11050#discussion_r133829303
Tree-SHA512: e72f696011d20acc0778e996659e41f9426bffce387b29ff63bf59ad1163d5146761e4445b2b9b9e069a80596a57c7f4402b75a15d5d20f69f775ae558cf67e9
40a0f9f Enable devirtualization opportunities by using the final specifier (C++11) (practicalswift)
9a1675e optim: mark a few classes final (Cory Fields)
Pull request description:
Using gcc's ```-Wsuggest-final-types``` and lto, I identified a few easy devirtualization wins:
> wallet/wallet.h:651:7: warning: Declaring type 'struct CWallet' final would enable devirtualization of 26 calls [-Wsuggest-final-types]
>coins.h:201:7: warning: Declaring type 'struct CCoinsViewCache' final would enable devirtualization of 13 calls [-Wsuggest-final-types]
>txdb.h:67:7: warning: Declaring type 'struct CCoinsViewDB' final would enable devirtualization of 5 calls [-Wsuggest-final-types]
>zmq/zmqnotificationinterface.h:16:7: warning: Declaring type 'struct CZMQNotificationInterface' final would enable devirtualization of 4 calls [-Wsuggest-final-types]
>httpserver.cpp:42:7: warning: Declaring type 'struct HTTPWorkItem' final would enable devirtualization of 2 calls [-Wsuggest-final-types]
Tree-SHA512: 2a825fd27121ccabaacff5cde2fc8a50d1b4cc846374606caa2a71b0cd8fcb0d3c9b5b3fd342d944998610e2168048601278f8a3709cc515191a0bb2d98ba782
e53615b Remove vchDefaultKey and have better first run detection (Andrew Chow)
Pull request description:
Removes vchDefaultKey which was only used for first run detection. Improves wallet first run detection by checking to see if any keys were read from the database.
This also fixes a (rather contrived) case where an encrypted non-HD wallet has corruption such that the default key is no longer valid and is loaded into a Core version that supports HD wallets. This causes a runtime exception since a new hd master key is generated as the software believes the wallet file is newly created but cannot add the generated key to the wallet since it is encrypted. I was only able to replicate this error by creating a non-hd wallet, encrypting it, then editing the wallet using `db_dump` and `db_load` before loading the wallet with hd enabled. This problem has been reported by [two](https://bitcointalk.org/index.php?topic=1993244.0) [users](https://bitcointalk.org/index.php?topic=1746976.msg17511261#msg17511261) so it is something that can happen, although that raises the question of "what corrupted the default key".
~P.S. I don't know what's up with the whitespace changes. I think my text editor is doing something stupid but I don't think those are important enough to attempt undoing them.~ Undid those
Tree-SHA512: 63b485f356566e8ffa033ad9b7101f7f6b56372b29ec2a43b947b0eeb1ada4c2cfe24740515d013aedd5f51aa1890dfbe499d2c5c062fc1b5d272324728a7d55
67ceff4 [wallet] Add logging to MarkReserveKeysAsUsed (John Newbery)
1221f60 [wallet] Remove keypool_topup_cleanups (John Newbery)
Pull request description:
A couple of minor cleanups suggested by @ryanofsky here: https://github.com/bitcoin/bitcoin/pull/11022#pullrequestreview-55598940
Does not affect functionality. Not required for v0.15.
Tree-SHA512: d8d0698fd26ea49a4157e68669d5511095760c3a1ecfa3f917e3f273efbafb55c51a202d677614216eae3f796b6e8d17506b2ec2d4799a94f18981b396e65eec
8f2f1e0 wallet: Avoid second mapWallet lookup (João Barbosa)
Pull request description:
All calls to `mapWallet.count()` have the intent to detect if a `txid` exists and most are followed by a second lookup to retrieve the `CWalletTx`.
This PR replaces all `mapWallet.count()` calls with `mapWallet.find()` to avoid the second lookup.
Tree-SHA512: 96b7de7f5520ebf789a1aec1949a4e9c74e13683869cee012f717e5be8e51097d068e2347a36e89097c9a89f1ed1a1529db71760dac9b572e36a3e9ac1155f29
6e8c48dc5 Add const to methods that do not modify the object for which it is called (practicalswift)
Pull request description:
Tree-SHA512: a6888111ba16fb796e320e60806e1a77d36f545989b5405dc7319992291800109eab0b8e8c286b784778f41f1ff5289e7cb6b4afd7aec77f385fbcafc02cffc1
Removes vchDefaultKey which was only used for first run detection.
Improves wallet first run detection by checking to see if any keys
were read from the database.
This will now also check for a valid defaultkey for backwards
compatibility reasons and to check for any corruption.
Keys will stil be generated on the first one, but there won't be
any shown in the address book as was previously done.
Only change in behavior is that unsupported combinations of parameters now
trigger more specific error messages instead of the vague "JSON value is not a
string as expected" error.
