faab63111d8f27335aa1f09c1a48da3be45135de shutdown: Stop threads before resetting ptrs (MarcoFalke)
Pull request description:
On shutdown some threads would continue to run after or during a pointer reset. This leads to occasional segfaults on shutdown.
Fix this by resetting the smart pointers after all threads that might read from them have been stopped.
This should fix:
* A segfault in the txindex thread, that occurs when the txindex destructor is done, but the thread was not yet stopped (as this is done in the base index destructor)
* A segfault in the scheduler thread, which dereferences conman. (e.g. CheckForStaleTipAndEvictPeers)
Tree-SHA512: abbcf67fadd088e10fe8c384fadfb90bb115d5317145ccb5363603583b320efc18131e46384f55a9bc574969013dfcbd08c49e0d42c004ed7212eca193858ab2
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
The way it was implemented caused vPendingMasternodes entries to be popped
but not necessarily connected to when at the same time quorum connections
were pending.
* Introduce SynchronousUpdatedBlockTip signal
This version of UpdatedBlockTip mirrors the asynchronous behavior that we
had before the introduction of asynchronous signal handling.
* Fix tab spacing in validationinterface.cpp
* Invoke CDeterministicMNManager::UpdatedBlockTip from validation thread
It must be invoked synchronously as otherwise things become inconsistent.
* Call CActiveMasternodeManager::Init with block index
pindexNew in UpdatedBlockTip is not necessarily the current tip, so we
shouldn't rely on it in Init(). This is due to the async nature of the
UpdatedBlockTip invocation.
* Use item data to figure out PS rounds for the input selected in CoinControl dialog
* Access GetRealOutpointPrivateSendRounds through WalletModel in CoinControl dialog
Instead of accessing the walet directly
* Make OverviewPage::privateSendStatus() mockable
GetTimeMillis() -> GetTime()
* Refactor PS related code in overviewpage to access wallet via wallet model
* 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>
bf64c3cb3 Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos)
04f78ab5b Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos)
fd849e1b0 Change AcceptToMemoryPool function signature (Alex Morcos)
Pull request description:
First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior.
Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](https://github.com/bitcoin/bitcoin/pull/9602#issue-202197849)
Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in #11303.
Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c
fix 11309
Signed-off-by: Pasta <pasta@dashboost.org>
fix &
Signed-off-by: Pasta <pasta@dashboost.org>
2e9406c Interrupt loading thread after shutdown request (João Barbosa)
Pull request description:
This change (currently) avoids loading the mempool if shutdown is requested.
Tree-SHA512: 3dca3a6ea5b09bd71db0974584d93dfe81819bc0bdbb4d9b6fa0474755306d1403f6c058ecb8211384493a8f7ca3a9134173db744b7344043cfc7d79286c8fd4
a8b5d20 Reset pblocktree before deleting LevelDB file (Sjors Provoost)
Pull request description:
#11043 repaced:
```
delete pblocktree;
pblocktree = new CBlockTreeDB(nBlockTreeDBCache, false, fReset);
```
With:
```
pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset));
```
This is problematic because `new CBlockTreeDB` tries to delete the existing file, which will fail with `LOCK: already held by process` if it's still open. That's the case for QT.
When QT finds a problem with the index it will ask the user if they want to reindex. At that point it has already opened `blocks/index`. It then runs this [while loop](https://github.com/bitcoin/bitcoin/blob/v0.16.0rc3/src/init.cpp#L1415) again with `fReset = 1`, resulting in the above error.
This change makes that error go away, presumably because `reset()` without an argument closes the file.
Tree-SHA512: fde8b546912f6773ac64da8476673cc270b125aa2d909212391d1a2001b35c8260a8772126b99dfd76b39faaa286feb7c43239185fe584bd4dc2bc04a64044ce
02fc886 Add braces to meet code style on line-after-the-one-changed. (Matt Corallo)
85aa839 Hold mempool.cs for the duration of ATMP. (Matt Corallo)
Pull request description:
This resolves an issue where getrawmempool() can race mempool
notification signals. Intuitively we use mempool.cs as a "read
lock" on the mempool with cs_main being the write lock, so holding
the read lock intermittently while doing write operations is
somewhat strange.
This also avoids the introduction of cs_main in getrawmempool()
which reviewers objected to in the previous fix in #12273
Tree-SHA512: 29464b9ca3890010ae13b7dc1c53487cc2bc9c3cf3d32a14cb09c8aa33848f57959d8991ea096beebcfb72f062e4e1962f104aefe4252c7db87633bbfe4ab317
1e5d14b qt: Clarify some comments (Wladimir J. van der Laan)
f5a4c3d qt: Make sure splash screen is freed on AppInitMain fail (Wladimir J. van der Laan)
Pull request description:
The `splashFinished` event was never sent if AppInitMain fails, causing the splash screen to stick around, causing problems later.
This bug has existed for a while but is now trigging potential crashed because the splash screen subscribes to wallet events.
Meant to fix#12372.
Tree-SHA512: 192a7e3a528015e771d7860dd95fd7b772292fd8064abf2a3cf3a8ea0d375cd43a6e8ed37ca1a38962fe1410c934599e557adf6a8ef9d87ec7f61b6e5fd8db7e
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)
Pull request description:
The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
e5033a5c9b/src/corelib/thread/qthread.cpp (L412-L415)
Potential fix for https://github.com/bitcoin/bitcoin/issues/12372#issuecomment-363642332
This reverts #11831 for now and hopefully restores the previous behaviour.
Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
dd2de47 Fix fast-shutdown crash if genesis block was not loaded (Matt Corallo)
1c9394a Fix fast-shutdown hang on ThreadImport+GenesisWait (Matt Corallo)
Pull request description:
The second commit is a much simpler alternative fix for the issue fixed in #12349. To test I made ShutdownRequested() always StartShutdown() after a certain number of calls, which turned up one other hang, fixed in the first commit.
Tree-SHA512: 86bde6ac4b8b4e2cb99fff87dafeed02c0d9514acee6d94455637fb2da9ffc274b5ad31b0a6b9f5bd7b700ae35395f28ddb14ffc65ddda3619aa28df28a5607d
aac6bce112 test: Make ua_comment test pass on 0.16.0 (Wladimir J. van der Laan)
Pull request description:
The specific length of the uacomment is one shorter on `0.16.0` than on `0.15.99` causing the (stupid) test to fail.
This change makes `assert_start_raises_init_error` optionally take a regexp, so that the error message can be checked without being specific about the reported length.
Tree-SHA512: 1c5e9f1d0b36f004f8113c7e211e8281194ce8057869ac3702172d8b021df3c3aa3b8f79b9434678ed0fb7146f848802410647d434fc884aa200b6a5e26afe8b
fa0b3da36cd7cf0aada22f2d459296b81274c2f9 Squashed 'src/univalue/' changes from 7890db99d6..5a58a46671 (MarcoFalke)
Pull request description:
Only change is a performance improvement. See https://github.com/bitcoin-core/univalue/pull/21#issue-333858474 and https://github.com/bitcoin-core/univalue/pull/15#issue-186748173
ACKs for top commit:
jnewbery:
ACK fa439e88af944082875e1fdb1cd8bb5a74b88b56
laanwj:
ACK fa439e88af944082875e1fdb1cd8bb5a74b88b56
Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
54245985fb3c89d72e285c4db39d38ed2f5fb0de Squashed 'src/secp256k1/' changes from 0b70241850..b19c000063 (Pieter Wuille)
Pull request description:
It's been 1.5 years since our secp256k1 subtree was updated, while the upstream project has undergone a number of incremental improvements (performance, tests, build system fixes), plus gained the groundwork for batch verification.
As we're early in the 0.19 window, this seems like a good time to get these merged.
ACKs for commit 99df27:
fanquake:
utACK 99df276 the subtree merge, still need to test the actual changes.
laanwj:
utACK 99df276da
Tree-SHA512: 769a699366321635068ebfbd9d3f30f6e72401c4fcdc1fdc84e5b3fd888c3f01437748f6cd23a507ab47cf04c226cd504fd48aee654457c34bb106c9db7e5c09
126999d depends: fix zmq build with mingw < 4.0 (Cory Fields)
387879d [depends] ZeroMQ 4.2.2 (fanquake)
Pull request description:
Update depends ZeroMQ to 4.2.2, the release notes are available [here](https://github.com/zeromq/libzmq/releases).
We can drop both patches, as they have both been merged upstream (they actually had been for some time but just hadn't yet made it into a release).
`--without-documentation` is deprecated and has been replaced with `--without-docs`.
`--disable-perf` disables building performance measuring tools, which are enabled by default, see the libzmq [configure.ac](https://github.com/zeromq/libzmq/blob/master/configure.ac#L367).
Updated dependencies.md.
`--disable-curve-keygen` disable building the curve key generation tool. See [here](https://github.com/zeromq/libzmq/blob/master/configure.ac#L405).
Can someone on windows test that this is still working correctly. Maybe @achow101 ?
Tree-SHA512: c6c4b15f545b6de21648f05027b5500fca0e6b5b72e791ac9a0aa523c57f2feb5aae94e42531275dddd922e11e462a52f08be1118ba1629c3cae765b18e5d720
65e91f5ed [tests] Test that mempool rejects coinbase transactions (James O'Beirne)
Pull request description:
Neither the unit nor functional tests appear to cover rejecting a transaction from acceptance to the mempool on the basis of it being a coinbase. Seems like a decent thing to have a test for.
Tree-SHA512: 53af53c975cad5d7a21c443d71a1c0ced5c70a7799b75bb44d9b7dd6ab2afbcdcaab14571540efeb848f3a1daee5e1dd856530d8f2b50582595219a1c17555ff
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
dcfef27 cli: Reject arguments to -getinfo (Wladimir J. van der Laan)
Pull request description:
Currently it's possible to accidentally type e.g.
bitcoin-cli -getinfo getbalance
and get an answer which can be confusing; the trailing arguments are just ignored.
To avoid this, throw an error if the user provides arguments to
`-getinfo`.
Tree-SHA512: 3603e8fa852b884d1dd3b7462db40b092fe8b3390fd4384b4ee330315d797aff711e9f62990012fd4b5a55c8678734ba8497a5488a09ee6b65cf8a99017d6eb4
* Masternodes must have required services enabled
* Add a comment about missing bits
* Refactor *InitParameterInteraction parts to follow surrounding code logic
Changes:
- move all related param interactions to appropriate places;
- try to softly set required params, complain and fail to start later if smth is not ok.
* Drop redundant code
* Few tweaks for MakeCollateralAmounts
Fail early, avoid creating non-usable change outputs
* Adjust wallet logic to display "make collaterals" txes correctly when we create one using non-max amount
* Split GetDenominationsToString into DenominationToAmount/DenominationToString
* Refactor SelectPrivateCoins into SelectDenominatedAmounts and drop ConvertList
* Refactor SelectPSInOutPairsByDenominations
* Drop GetDenominationsBits
* Drop GetDenominations
* Address review comments
* Fix/bring back session denom randomization in StartNewQueue
Note: no need to randomize if there is only one single option
