004f999 boost: drop boost threads for [alert|block|wallet]notify (Cory Fields)
0827267 boost: drop boost threads from torcontrol (Cory Fields)
ba91724 boost: remove useless threadGroup parameter from Discover (Cory Fields)
f26866b boost: drop boost threads for upnp (Cory Fields)
Pull request description:
This doesn't completely get rid of boost::thread, but this batch should be easy to review, and leaves us with only threadGroup (scheduler + scriptcheck) remaining.
Note to reviewers: The upnp diff changes a bunch of whitespace, it's much more clear with 'git diff -w'
Tree-SHA512: 5a356798d0785f93ed143d1f0afafe890bc82f0d470bc969473da2d2aa78bcb9b096f7ba11b92564d546fb447d4bd0d347e7842994ea0170aafd53fda7e0a66e
669c943 Avoid leaking prioritization information when relaying transactions (Suhas Daftuar)
e868b22 fee estimator: avoid sorting mempool on shutdown (Suhas Daftuar)
0975406 Correct mempool mapTx comment (Suhas Daftuar)
Pull request description:
Following up on #12127 and #12118, this cleans up a comment that was left incorrect in txmempool.h, and addresses a couple of the observations @TheBlueMatt made about an unnecessary use of `queryHashes()` and a small information leak when prioritizing transactions.
Left undone is nuking queryHashes altogether; that would require changing the behavior of the `getrawmempool` rpc call, which I think I might be in favor of doing, but wanted to save for its own PR.
Tree-SHA512: c97d10b96dcd6520459287a4a2eda92774173757695100fcfe61e526aef86f394507c331d17f9e0c14b496c33ec46198a0f165a847762ca50f7c6780b993f162
If the user somehow manages to get into ShutdownRequested before
ThreadImport gets to ActivateBestChain() we may hang waiting on
condvar_GenesisWait forever. A simple wait_for and
ShutdownRequested resolves this case.
Change `-conf`'s and others' help messages to indicate that relative path
values will be prefixed by the datadir path. This behavior is confusing when
attempting to specify a configuration file in the current directory with
`-conf=bitcoin.conf`, but loading the `bitcoin.conf` file in ~/.bitcoin
datadir.
082a61c Move scheduler/threadGroup into common-init instead of per-app (Matt Corallo)
Pull request description:
This resolves#12229 which pointed out a shutdown deadlock due to
scheduler/checkqueue having been shut down while network message
processing is still running.
Tree-SHA512: 0c0a76113996b164b0610d3b8c40b396f3e384d165bf098768e31fe3701b00763d0d810ef24702387e2e936fefb9fb900a6225f7417bb0175b585f365d542660
34328b4 Use PACKAGE_NAME instead of hardcoding application name in log message (Wladimir J. van der Laan)
0c74e2e Log debug build status and warn when running benchmarks (Wladimir J. van der Laan)
Pull request description:
Log whether the starting instance of bitcoin core is a debug or release build (--enable-debug).
Also warn when running the benchmarks with a debug build, to prevent mistakes comparing debug to non-debug results.
Tree-SHA512: f612dcb7d0a8435016cff0df8aef4942144dfb88be8a00df45cc8830d2aba4b167f6d397b83f8f57d57685888babd04ba88d4dac5a202d3dbd91bcbea3708ef0
This resolves#12229 which pointed out a shutdown deadlock due to
scheduler/checkqueue having been shut down while network message
processing is still running.
Also warn if bitcoind is configured to use a relative -datadir path.
Specifying paths relative to the current working directory in a daemon process
can be dangerous, because files can fail to be located even if the
configuration doesn't change, but the daemon is started up differently.
Specifying a relative -datadir now adds a warning to the debug log. It would
not be backwards-compatible to forbid relative -datadir paths entirely, and it
could also be also inconvenient for command line testing.
Specifying a relative -walletdir now results in a startup error. But since the
-walletdir option is new in 0.16.0, there should be no compatibility issues.
Another reason not to use working directory paths for -walletdir specifically
is that the default -walletdir is a "wallets" subdirectory inside the datadir,
so it could be surprising that setting -walletdir manually would choose a
directory rooted in a completely different location.
Log whether the starting instance of bitcoin core is a debug or release
build (--enable-debug).
Also warn when running the benchmarks with a debug build, to prevent
mistakes comparing debug to non-debug results.
07c4838 Always return true if AppInitMain got to the end (Matt Corallo)
Pull request description:
This should fix a rare zapwallettxes failure on travis, but also
avoids having init operations (re-adding wallet transactions to
mempool) running after RPC is free'd.
I believe this was the failure at https://travis-ci.org/bitcoin/bitcoin/jobs/311747844 (from #11605).
Tree-SHA512: f0fea8c1b9265e2eeda57043d541380a3e58e4d9388fa24628a52fd56324257fcd7df0ca02e8f77f66fadd68d951893bab0f610ed9fd0a89b2ccd6bad1efa351
This should fix a very rare travis failure in zapwallettxes, but
is also more correct, as you can currently race
ReacceptWalletTransactions with stop RPC calls to get bitcoind to
(IMO) eroneously return a non-0 exit code.
This patch adds an option to configure the name and/or directory of the
debug log.
The user can specify either a relative path, in which case the path
is relative to the data directory. They can also specify an absolute
path to put the log anywhere else in the file system.
d31e5c1 Fix shutdown in case of errors during initialization (Wladimir J. van der Laan)
Pull request description:
PR #10286 introduced a few steps which are not robust to early shutdown in initialization.
Stumbled upon this with #11781, not sure if there are other scenarios that can trigger it, but it's good to harden against this in any case.
E.g.
```
$ src/bitcoind -debuglogfile=/dfdf
Error: Could not open debug log file /dfdf
Program received signal SIGSEGV, Segmentation fault.
UnregisterValidationInterface (pwalletIn=0x0) at /.../bitcoin/src/validationinterface.cpp:82
82 g_signals.m_internals->BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
(gdb) bt
#0 UnregisterValidationInterface (pwalletIn=0x0) at /.../bitcoin/src/validationinterface.cpp:82
#1 0x00005555555a11fc in Shutdown () at /.../bitcoin/src/init.cpp:196
#2 0x00005555555961cc in AppInit (argc=<optimized out>, argv=<optimized out>) at /.../bitcoin/src/bitcoind.cpp:183
#3 0x0000555555596249 in main (argc=0, argv=0x555555ecf200) at /.../bitcoin/src/bitcoind.cpp:19
```
Tree-SHA512: 7dd9570a9803514a17781bfadf1edde47e96df4e852cce2f423cab422e005fb94d44e777af1a6ea5167b04a4d889e848ae7a61a7e0e94232247ddea32ee70fc8
c60c49b Improve help text and behavior of RPC-logging (Akio Nakamura)
Pull request description:
1. It is allowed `libevent` logging to be updated during runtime,
but still described that restriction in the help text.
So we delete these text.
2. Add a descrption about the evaluation order of `<include>` and
`<exclude>` to clarify how debug loggig categories to be set.
3. Add a description about the available logging category `"all"`
which is not explained.
4. Add `"optional"` to the help text of `<include>` and `<exclude>`.
5. Add missing new lines before `"Argument:"`.
6. `"0"`,`"1"` are allowed in both array of `<include>` and `<exclude>`.
`"0"` is **ignored** and `"1"` is treated **same as** `"all"`.
It is confusing, so forbid them.
7. It always returns all logging categories with status.
Fix the help text to match this behavior.
Tree-SHA512: c2142da1a9bf714af8ebc38ac0d82394e2073fc0bd56f136372e3db7b2af3b6746f8d6b0241fe66c1698c208c124deb076be83f07dec0d0a180ad150593af415
PR #10286 introduced a few steps which are not robust to early shutdown
in initialization.
Stumbled upon this with #11781, not sure if there are other scenarios
that can trigger it, but it's harden against this in any case.
abbd230 Move RPC registration out of AppInitParameterInteraction (Russell Yanofsky)
Pull request description:
Move to AppInitServers. This doesn't have any effects on bitcoin behavior. It was just strange to have this unrelated code in the middle of parameter interaction.
Tree-SHA512: 373e18f2ef8d21999ad36295d69326128a3086044acfc8ed537abd5497c8d3620b9832f7f6aa87c0c0939bb5e0d92be8a3c006b5997e9e6fa20334f5610c89bc
A) The changes in behavior are as follows:
1. Introduce logging category "none" as alias of "0" for
both RPC-logging and bitcoind "-debug" parameter.
2. Same as "0" is given to argument of "-debug",
if "none" or "0" is given to <include>, all other given logging
categories are ignored. The same is true for <exclude>.
(Before this PR, "0" was accepted but just be ignored itself.)
B) The changes in the help text are as follows:
1. Add a descrption about the evaluation order of <include> and
<exclude> to clarify how debug loggig categories to be set.
2. Delete text that describe restriction about libevent because
it's already allowed libevent logging to be updated during runtime.
3. Add a description for category "all", "1", "none" and "0".
4. Add "optional" to the help text of <include> and <exclude>.
5. Add missing new lines before "Argument:".
6. This RPC always returns all logging categories with status.
Fix the help text to match this behavior.
89f0312 Remove redundant pwallet nullptr check (Matt Corallo)
c4784b5 Add a dev notes document describing the new wallet RPC blocking (Matt Corallo)
3ea8b75 Give ZMQ consistent order with UpdatedBlockTip on scheduler thread (Matt Corallo)
cb06edf Fix wallet RPC race by waiting for callbacks in sendrawtransaction (Matt Corallo)
e545ded Also call other wallet notify callbacks in scheduler thread (Matt Corallo)
17220d6 Use callbacks to cache whether wallet transactions are in mempool (Matt Corallo)
5d67a78 Add calls to CWallet::BlockUntilSyncedToCurrentChain() in RPCs (Matt Corallo)
5ee3172 Add CWallet::BlockUntilSyncedToCurrentChain() (Matt Corallo)
0b2f42d Add CallFunctionInQueue to wait on validation interface queue drain (Matt Corallo)
2b4b345 Add ability to assert a lock is not held in DEBUG_LOCKORDER (Matt Corallo)
0343676 Call TransactionRemovedFromMempool in the CScheduler thread (Matt Corallo)
a7d3936 Add a CValidationInterface::TransactionRemovedFromMempool (Matt Corallo)
Pull request description:
Based on #10179, this effectively reverts #9583, regaining most of the original speedups of #7946.
This concludes the work of #9725, #10178, and #10179.
See individual commit messages for more information.
Tree-SHA512: eead4809b0a75d1fb33b0765174ff52c972e45040635e38cf3686cef310859c1e6b3c00e7186cbd17374c6ae547bfbd6c1718fe36f26c76ba8a8b052d6ed7bc9
76ea17c79 Add mutex requirement for AddToCompactExtraTransactions(…) (practicalswift)
4616c825a Use -Wthread-safety-analysis if available (+ -Werror=thread-safety-analysis if --enable-werror) (practicalswift)
7e319d639 Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. (Matt Corallo)
Pull request description:
* Add mutex requirement for `AddToCompactExtraTransactions(…)`.
* Use `-Wthread-safety-analysis` if available.
* Rebased on top of https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923 - now includes: Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost.
Tree-SHA512: fb7365f85daa2741c276a1c899228181a8d46af51db7fbbdffceeaff121a3eb2ab74d7c8bf5e7de879bcc5042d00d24cb4649c312d51caba45a3f6135fd8b38f
Commit 1.
This code was written by @TheBlueMatt in the following branch:
* https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923
This commit message was written by me (@practicalswift) who also squashed
@TheBlueMatt's commits into one and tried to summarize the changes made.
Commit 2.
Remove boost include. Remove boost mentions in comments.
Move to AppInitServers. This doesn't have any effects on bitcoin behavior. It
was just strange to have this unrelated code in the middle or parameter
interaction.
If our tip hasn't updated in a while, that may be because our peers are
not relaying blocks to us that we would consider valid. Allow connection
to an additional outbound peer in that circumstance.
Also, periodically check to see if we are exceeding our target number of
outbound peers, and disconnect the one which has least recently
announced a new block to us (choosing the newest such peer in the case
of tie).
f4c4e38 [trivial] Make namespace explicit for is_regular_file (John Newbery)
Pull request description:
is_regular_file resolves using argument dependent lookup. Make the
namespace explicit so it's obvious where the function is defined.
For those not familiar with argument dependent lookups:
- http://en.cppreference.com/w/cpp/language/adl
- https://en.wikipedia.org/wiki/Argument-dependent_name_lookup
Thanks to C++ guru @ryanofsky for pointing this out to me.
Tree-SHA512: 919f1818081a8f90c5751181f87e13b06d90f8aec0ab873100434e55c85cca6e0e288ecc7f135e19e9b5dba7952e96b6393864b7840e20b69dd40e92a157928b
This is currently unused, but will by used by wallet to cache when
transactions are in the mempool, obviating the need for calls to
mempool from CWalletTx::InMempool()
15f5d3b17 Switch DNSSeed-needed metric to any-automatic-nodes, not services (Matt Corallo)
5ee88b4bd Clarify docs for requirements/handling of addnode/connect nodes (Matt Corallo)
57edc0b0c Rename fAddnode to a more-descriptive "manual_connection" (Matt Corallo)
44407100f Replace relevant services logic with a function suite. (Matt Corallo)
Pull request description:
This was mostly written as a way to clean things up so that the NETWORK_LIMITED PR (#10387) can be simplified a ton, but its also a nice standalone cleanup that will also require a bit of review because it tweaks a lot of stuff across net. The new functions are fine in protocol.h right now since they're straight-forward, but after NETWORK_LIMITED will really want to move elsewhere after @theuni moves the nServices-based selection to addrman from connman.
Adds HasAllRelevantServices and GetRelevantServices, which check
for NETWORK|WITNESS.
This changes the following:
* Removes nRelevantServices from CConnman, disconnecting it a bit
more from protocol-level logic.
* Replaces our sometimes-connect-to-!WITNESS-nodes logic with
simply always requiring WITNESS|NETWORK for outbound non-feeler
connections (feelers still only require NETWORK).
* This has the added benefit of removing nServicesExpected from
CNode - instead letting net_processing's VERSION message
handling simply check HasAllRelevantServices.
* This implies we believe WITNESS nodes to continue to be a
significant majority of nodes on the network, but also because
we cannot sync properly from !WITNESS nodes, it is strange to
continue using our valuable outbound slots on them.
* In order to prevent this change from preventing connection to
-connect= nodes which have !WITNESS, -connect nodes are now
given the "addnode" flag. This also allows outbound connections
to !NODE_NETWORK nodes for -connect nodes (which was already true
of addnodes).
* Has the (somewhat unintended) consequence of changing one of the
eviction metrics from the same
sometimes-connect-to-!WITNESS-nodes metric to requiring
HasRelevantServices.
This should make NODE_NETWORK_LIMITED much simpler to implement.
Tree-SHA512: 90606896c86cc5da14c77843b16674a6a012065e7b583d76d1c47a18215358abefcbab44ff4fab3fadcd39aa9a42d4740c6dc8874a58033bdfc8ad3fb5c649fc
Adds HasAllRelevantServices and GetRelevantServices, which check
for NETWORK|WITNESS.
This changes the following:
* Removes nRelevantServices from CConnman, disconnecting it a bit
more from protocol-level logic.
* Replaces our sometimes-connect-to-!WITNESS-nodes logic with
simply always requiring WITNESS|NETWORK for outbound non-feeler
connections (feelers still only require NETWORK).
* This has the added benefit of removing nServicesExpected from
CNode - instead letting net_processing's VERSION message
handling simply check HasAllRelevantServices.
* This implies we believe WITNESS nodes to continue to be a
significant majority of nodes on the network, but also because
we cannot sync properly from !WITNESS nodes, it is strange to
continue using our valuable outbound slots on them.
* In order to prevent this change from preventing connection to
-connect= nodes which have !WITNESS, -connect nodes are now
given the "addnode" flag. This also allows outbound connections
to !NODE_NETWORK nodes for -connect nodes (which was already true
of addnodes).
* Has the (somewhat unintended) consequence of changing one of the
eviction metrics from the same
sometimes-connect-to-!WITNESS-nodes metric to requiring
HasRelevantServices.
This should make NODE_NETWORK_LIMITED much simpler to implement.
c626dcb50 Make fUseCrypto atomic (MeshCollider)
731065b11 Consistent parameter names in txdb.h (MeshCollider)
35aeabec6 Make fReindex atomic to avoid race (MeshCollider)
58d91af59 Fix race for mapBlockIndex in AppInitMain (MeshCollider)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/11106
Also makes fReindex atomic as suggested in @TheBlueMatt comment below, and makes fUseCrypto atomic as suggested in 10916
d291e7635b just renames the parameters in the txdb header file to make them consistent with those used in the cpp file, noticed it when looking for uses of fReindex
Tree-SHA512: b378aa7289fd505b76565cd4d48dcdc04ac5540283ea1c80442170b0f13cb6df771b1a94dd54b7fec3478a7b4668c224ec9d795f16937782724c5d020edd3a42
cffe85f Skip sys::system(...) call in case of empty command (practicalswift)
6fb8f5f Check that -blocknotify command is non-empty before executing (practicalswift)
Pull request description:
Check that `-blocknotify` command is non-empty before executing.
To make the `BlockNotifyCallback(...)` (`-blocknotify`) behaviour consistent with that of:
* `AlertNotify(...)` (`-alertnotify`)
* `AddToWallet(...)` (`-walletnotify`)
Tree-SHA512: 18272166793a5a8b9cc2a727bfbcea53d38c329a55bc975c02db601329d608a61c20e026ce4b616193ecd3810dca4d3e2cb3bf773898a51872008a8dba96763e
048e0c3e2 [rpc] [tests] Add deprecated RPC test (Cristian Mircea Messel)
d4cdbd6fb [rpc] Deprecate estimatefee RPC (John Newbery)
Pull request description:
Deprecates estimatefee in v0.16, for final removal in v0.17.
This commit introduces a phased removal of RPC methods. RPC method is
disabled by default in version x, but can be enabled by using the
`-deprecatedrpc=<methodname>` argument. RPC method is removed entirely in version
(x+1).
This gives users fair warning that an RPC is to be removed, and time to change client software if necessary. Deprecation warnings in RPC return values or release notes are easily ignored.
This is a more generic version of the approach I tried to use in #10841, which too late to make it into v0.15.
Tree-SHA512: 9695a600e84b812974387333e4a6805d18972da30befb754e9e4da77cd9815d00c5cc2ee0b0350bdbbdb5fdc6ba47789f8b2c6f5b15c8cd5a1deefcc4832da30
Deprecate estimatefee in v0.16, for final removal in v0.17.
This commit introduces a phased removal of RPC methods. RPC method is
disabled by default in version x, but can be enabled by using the
`-deprecatedrpc=<method>` argument. RPC method is removed entirely in
version (x+1).
* This removes block-size-limiting code in favor of GBT clients
doing the limiting themselves (if at all).
* -blockmaxsize is deprecated and only used to calculate an implied
blockmaxweight, addressing confusion from multiple users.
* getmininginfo's currentblocksize return value was returning
garbage values, and has been removed, also removing a
GetSerializeSize call in some block generation inner loops and
potentially addressing some performance edge cases.
