362ded410b Avoid using g_rpc_node global in wallet code (Russell Yanofsky)
8922d7f6b7 scripted-diff: Remove g_connman, g_banman globals (Russell Yanofsky)
e6f4f895d5 Pass NodeContext, ConnMan, BanMan references more places (Russell Yanofsky)
4d5448c76b MOVEONLY: Move NodeContext struct to node/context.h (Russell Yanofsky)
301bd41a2e scripted-diff: Rename InitInterfaces to NodeContext (Russell Yanofsky)
Pull request description:
This change is mainly a naming / organization change intended to simplify #10102. It:
- Renames struct InitInterfaces to struct NodeContext and moves it from
src/init.h to src/node/context.h. This is a cosmetic change intended to make
the point of the struct more obvious.
- Gets rid of BanMan and ConnMan globals making them NodeContext members
instead. Getting rid of these globals has been talked about in past as a way
to implement testing and simulations. Making them NodeContext members is a
way of keeping them accessible without the globals.
- Splits g_rpc_interfaces global into g_rpc_node and g_rpc_chain globals. This
better separates node and wallet rpc methods. Node RPC methods should have
access NodeContext, while wallet RPC methods should only have indirect access
to node functionality via interfaces::Chain.
- Adds NodeContext& references to interfaces::Chain class and the
interfaces::MakeChain() function. This is needed to access ConnMan and BanMan
instances without the globals.
- Gets rid of redundant Node and Chain instances in Qt tests. This is
needed due to the previous MakeChain change, and also makes test setup a
little more straightforward. More cleanup could be done in the future, but it
will require deduplication of bitcoind, bitcoin-qt, and TestingSetup init
code.
ACKs for top commit:
laanwj:
ACK 362ded410b
Tree-SHA512: 9ae6ff1e33423291d1e52056bac95e0874538390892a6e83c4c115b3c73155a8827c0191b46eb3d14e3b3f6c23ccb08095490880fbc3188026319c71739f7db2
f201ba59ff Refactor: Split up CWallet and LegacyScriptPubKeyMan and classes (Andrew Chow)
6702048f91 MOVEONLY: Move key handling code out of wallet to keyman file (Andrew Chow)
ab053ec6d1 Move wallet enums to walletutil.h (Andrew Chow)
Pull request description:
Moves key management functions into a new class LegacyScriptPubKeyMan. First two commits are move-only commits which move stuff out of wallet.{h/cpp} and into newly created scriptpubkeyman.{h/cpp}. Third commit changes several things in CWallet to use LegacyScriptPubKeyMan.
First step in the wallet boxes refactor. Note that LegacyScriptPubKeyMan and ScriptPubKeyMan cannot be used standalone yet and are still very much tied into CWallet with both accessing functions within each other. This PR is to help reduce review burden.
ACKs for top commit:
Sjors:
Code review ACK f201ba5.
promag:
Code review ACK f201ba59ff.
ryanofsky:
Code review ACK f201ba59ff
MarcoFalke:
ACK f201ba59ff
Tree-SHA512: bdc0d8595a06233fe003afcf968a38e0e8cc584a6a89c5bcd05309ac29dca852391802d46763ef81a108d146d0f40c79ea5438e87234ed12b4b8360c9aec94c0
This moves CWallet members and methods dealing with keys to a new
LegacyScriptPubKeyMan class, and updates calling code to reference the new
class instead of CWallet.
Most of the changes are simple text replacements and variable substitutions
easily verified with:
git log -p -n1 -U0 --word-diff-regex=.
The only nontrivial chunk of code added is the new LegacyScriptPubKeyMan class
declaration, but this code isn't new and is just selectively copied and moved
from the previous CWallet class declaration. This can be verified with:
git log -p -n1 --color-moved=dimmed_zebra src/wallet/scriptpubkeyman.h src/wallet/wallet.h
or
git diff HEAD~1:src/wallet/wallet.h HEAD:src/wallet/scriptpubkeyman.h
This commit does not change behavior.
QApplication takes the command line arguments and parses them itself
for some built in command line arguments that it has. We don't want
any of those built in arguments, so instead give it dummy arguments.
9a841696c1 tests: Reduce compilation time and unneccessary recompiles by removing unused includes in tests (practicalswift)
Pull request description:
Reduce compilation time and unneccessary recompiles by removing unused includes in tests.
A subset of #16273 ("refactor: Reduce total compilation time by 2% and avoid unnecessary recompiles by removing unused includes") as requested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/16273#issuecomment-505022643.
ACKs for top commit:
Sjors:
ACK 9a84169 on macOS 10.14.5 (I rebased on #16289)
Tree-SHA512: bcb6ecffef689a9839bee1a5cb93abe83db1f30819a54226c5630fee456b5a5d187507d06861454adfda939c3556a975113f97662e415cb47fa0327ea4fd09fb
78e407ad0c GetKeyBirthTimes should return key ids, not destinations (Gregory Sanders)
70946e7fee Replace CScriptID and CKeyID in CTxDestination with dedicated types (Gregory Sanders)
Pull request description:
The current usage seems to be an overloading of meanings. `CScriptID` is used in the wallet as a lookup key, as well as a destination, and `CKeyID` likewise. Instead, have all destinations be dedicated types.
New types:
`CScriptID`->`ScriptHash`
`CKeyID`->`PKHash`
ACKs for commit 78e407:
ryanofsky:
utACK 78e407ad0c. Only changes are removing extra CScriptID()s and fixing the test case.
Sjors:
utACK 78e407a
meshcollider:
utACK 78e407ad0c
Tree-SHA512: 437f59fc3afb83a40540da3351507aef5aed44e3a7f15b01ddad6226854edeee762ff0b0ef336fe3654c4cd99a205cef175211de8b639abe1130c8a6313337b9
Though at the moment ChainActive() simply references `g_chainstate.m_chain`,
doing this change now clears the way for multiple chainstate usage and allows
us to script the diff.
-BEGIN VERIFY SCRIPT-
git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/\(::\)\{0,1\}chainActive/::ChainActive()/g'
-END VERIFY SCRIPT-
faf400077d scripted-diff: Bump copyright headers in test, bench (MarcoFalke)
fa821904bf scripted-diff: Rename test_bitcoin to test/setup_common (MarcoFalke)
fa8685d49e test: Use test_bitcoin setup in bench, Add test utils (MarcoFalke)
666696b673 test: Have segwit always active in (Basic)TestingSetup (MarcoFalke)
Pull request description:
Now that the fuzz tests can use the BasicTestingSetup [1], do the same for bench.
Also move some duplicate code to a common "test/util" module.
[1]: fuzz: Link BasicTestingSetup (shared with unit tests) #15504
ACKs for commit faf400:
jonatack:
ACK faf400077d
Tree-SHA512: 8ac5692e72cf50e460958f291643ae6b8bb04d5c1331ed50dce9eb4e9457e5a925144c532c42b360a26707e11eeece74aab27db8c76ab9a429b9dd7167e7cdc4
44de1561a Remove remaining chainActive references from CWallet (Russell Yanofsky)
db21f0264 Convert CWallet::ScanForWalletTransactions and SyncTransaction to the new Chain apis (Russell Yanofsky)
2ffb07929 Add findFork and findBlock to the Chain interface (Russell Yanofsky)
d93c4c1d6 Add time methods to the Chain interface (Russell Yanofsky)
700c42b85 Add height, depth, and hash methods to the Chain interface (Russell Yanofsky)
Pull request description:
This change removes uses of `chainActive` and `mapBlockIndex` globals in wallet code. It is a refactoring change which does not affect external behavior.
This is the next step in the larger #10973 refactoring change, which removes all other accesses to node global variables from wallet code. Doing this is useful to provide a better defined interface between the wallet and node, and necessary to allow wallet and node code to run in separate processes in #10102.
Tree-SHA512: 4dcec8a31c458f54e2ea6ecf01e430469b0994c5b41a21a2d150efa67cd209f4c93ae210a101e064b3a87c52c6edfc70b070e979992be0e3a00fd425de6230a8
223de8d94d Document RNG design in random.h (Pieter Wuille)
f2e60ca985 Use secure allocator for RNG state (Pieter Wuille)
cddb31bb0a Encapsulate RNGState better (Pieter Wuille)
152146e782 DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille)
a1f252eda8 Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille)
4ea8e50837 Remove hwrand_initialized. (Pieter Wuille)
9d7032e4f0 Switch all RNG code to the built-in PRNG. (Pieter Wuille)
16e40a8b56 Integrate util/system's CInit into RNGState (Pieter Wuille)
2ccc3d3aa3 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille)
aae8b9bf0f Add thread safety annotations to RNG state (Pieter Wuille)
d3f54d1c82 Rename some hardware RNG related functions (Pieter Wuille)
05fde14e3a Automatically initialize RNG on first use. (Pieter Wuille)
2d1cc50939 Don't log RandAddSeedPerfmon details (Pieter Wuille)
6a57ca91da Use FRC::randbytes instead of reading >32 bytes from RNG (Pieter Wuille)
Pull request description:
This does not remove OpenSSL, but makes our own PRNG the 'main' one; for GetStrongRandBytes, the OpenSSL RNG is still used (indirectly, by feeding its output into our PRNG state).
It includes a few policy changes (regarding what entropy is seeded when).
Before this PR:
* GetRand*:
* OpenSSL
* GetStrongRand*:
* CPU cycle counter
* Perfmon data (on Windows, once 10 min)
* /dev/urandom (or equivalent)
* rdrand (if available)
* From scheduler when idle:
* CPU cycle counter before and after 1ms sleep
* At startup:
* CPU cycle counter before and after 1ms sleep
After this PR:
* GetRand*:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* GetStrongRand*:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* /dev/urandom (or equivalent)
* OpenSSL
* CPU cycle counter again
* From scheduler when idle:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter before and after 1ms sleep
* Perfmon data (on Windows, once every 10 min)
* At startup:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* /dev/urandom (or equivalent)
* OpenSSL
* CPU cycle counter again
* Perfmon data (on Windows, once every 10 min)
The interface of random.h is also simplified, and documentation is added.
This implements most of #14623.
Tree-SHA512: 0120e19bd4ce80a509b5c180a4f29497d299ce8242e25755880851344b825bc2d64a222bc245e659562fb5463fb7c70fbfcf003616be4dc59d0ed6534f93dd20
Only change in behavior is "Rescan started from block <height>" message
replaced by "Rescan started from block <hash>" message in
ScanForWalletTransactions.
Co-authored-by: Ben Woosley <ben.woosley@gmail.com>
There was only one place in the codebase where we're directly reading >32 bytes from
the RNG. One possibility would be to make the built-in RNG support large reads, but
using FastRandomContext lets us reuse code better.
There is no change in behavior here, because the FastRandomContext constructor
uses GetRandBytes internally.
bd3b0361d Add stop_block out arg to ScanForWalletTransactions (Ben Woosley)
3002d6cf3 Return a status enum from ScanForWalletTransactions (Ben Woosley)
bb24d6865 Make CWallet::ScanForWalletTransactions args and return value const (Ben Woosley)
Pull request description:
Return the failed block as an out arg.
Fixes #11450.
/cc #12275
Tree-SHA512: 6a523e5425ebfe24e664a942ae21c797ccc1281c25b1bf8d02ad95c19dae343fd8051985ef11853474de7628fd6bed5f15190fbc087c3466ce6fdecab37d72a9
Accurately reports the last block successfully scanned, replacing a return of
the chain tip, which represented possibly inaccurated data in a race condition.
This commit does not change behavior. All it does is pass new function
parameters.
It is easiest to review this change with:
git log -p -n1 -U0 --word-diff-regex=.
65f3672f3b wallet: Refactor to use WalletLocation (João Barbosa)
01a4c095c8 wallet: Add WalletLocation utility class (João Barbosa)
Pull request description:
Advantages of this change:
- avoid resolving wallet absolute path and name repetitively and in multiple places;
- avoid calling `GetWalletDir` in multiple places;
- extract these details from the actual wallet implementation.
The `WalletLocation` class can be a way to represent a wallet not yet loaded that exists in the wallet directory.
Tree-SHA512: 71ec09786e038499710e7acafe92d66ab9883fc894964e267443ae9c10a6872a10995c3987a169c436a4e793dae96b28fb97bd7f78483c4b72ac930fa23f8686
48439b3c10 Don't link SSL_LIBS with GUI unless BIP70 is enabled (James Hilliard)
fbb643d2a5 Add BIP70 deprecation warning (James Hilliard)
38b98507cd qt: cleanup: Move BIP70 functions together in paymentserver (Wladimir J. van der Laan)
9dcf6c0dfe build: Add --disable-bip70 configure option (Wladimir J. van der Laan)
Pull request description:
This is based off of #11622 and adds a deprecation warning when a BIP70 URL is used.
Rational:
- BIP70 increases attack surface in multiple ways and is difficult for third party wallets to implement in a secure manner
- Very few merchants use the standard BIP70 variant supported by Bitcoin Core
- The one major payment processor that doesn't support BIP21 and currently uses a customized non-standard version of BIP70 has indicated that "Unfortunately the original BIP70 is not useful for us."
Tree-SHA512: 1e16ee8d2cdac9499f751ee7b50d058278150f9e38a87a47ddb5105dd0353cdedabe462903f54ead6209b249b249fe5e6a10d29631531be27400f2f69c25b9b9
fadffae17f Revert "Make qt wallet test compatible with qt4" (MarcoFalke)
Pull request description:
This removes the file added in #10098
Tree-SHA512: 4aa887ec02aec943f3d74f469c935acf57e43ddbea6f979bb37d3a14670571fa2276cf578e8978de92b5854d8b462f24e5e0addfb27b92fbedf90726eb5d985e
a3197c5294 Disable wallet and address book Qt tests on macOS minimal platform (Russell Yanofsky)
Pull request description:
macOS minimal platform is frequently broken, and these are currently failing with Qt 5.11.1.
The tests do pass when run on the full cocoa platform (with `test_bitcoin-qt -platform cocoa`).
Stack trace from test crash: https://gist.github.com/ryanofsky/3401fb63c52d13d5585e7fc777361f1e
Tree-SHA512: a05644ef15d75ea7d7f85ea804c6a5fe78e4e7358b189cbab639d9f7dc46163a35f77f7a2b4ae2fd6be5b9fb22898386b4d88069d5ee8d5fdbd995157c6f0846
