32da92bdf6 gitian: Improve error handling (Wladimir J. van der Laan)
Pull request description:
Improve error handling in gitian builds:
- Set fail-on-error and pipefail flag, this causes a command to fail when either of the pipe stages fails, not only when the last of the stages fails, so this improves error detection.
- Also use `xargs` instead of `find -exec`, because `find` will not propagate errors in the executed command, but `xargs` will.
This will avoid some issues like #15541 where non-determinism is silently introduced due to errors caused by environment conditions (such as lack of disk space in that case).
Tree-SHA512: d5d3f22ce2d04a75e5c25e935744327c3adc704c2d303133f2918113573a564dff3d3243d5569a2b93ee7eb0e97f8e1b1ba81767e966af9015ea711a14091035
82c3b3f8e0 Remove sharp edge (uninitialized m_filter_type) when using the compiler-generated constructor for BlockFilter (practicalswift)
Pull request description:
Remove sharp edge (uninitialised member `m_filter_type`) when using the compiler-generated constructor for `BlockFilter`.
Before (but after added test):
```
$ src/test/test_bitcoin -t blockfilter_tests/blockfilter_basic_test
Running 1 test case...
test/blockfilter_tests.cpp(118): error: in "blockfilter_tests/blockfilter_basic_test": check default_ctor_block_filter_1.GetFilterType() == default_ctor_block_filter_2.GetFilterType() has failed [ != ]
*** 1 failure is detected in the test module "Bitcoin Test Suite"
```
After:
```
$ src/test/test_bitcoin -t blockfilter_tests/blockfilter_basic_test
Running 1 test case...
*** No errors detected
```
Tree-SHA512: 21d41f036b0bf12adcf1a788d84747353f2023cb85fd8ea6c97222967032e8bf54e7910cadb45dfcecd78e5b5dca86685f78cad0596b6d1a08f910ebf20d90aa
28c86de3b gui: Drop unused return values in WalletFrame (João Barbosa)
Pull request description:
This is a small cleanup since the return value of `WalletFrame` methods are not used. This is in line with the usual async slot declaration.
Tree-SHA512: ff0ca098804118bba200a58cd796ff90e853a6430e58125bd178b7bfa9b2b763c13d17b81e8f3ebd94395cac249d80379ba1529680c47682ba6a2ed81492ba33
519b0bc5dc Make last disconnected block BLOCK_FAILED_VALID, even when aborted (Pieter Wuille)
8d220417cd Optimization: don't add txn back to mempool after 10 invalidates (Pieter Wuille)
9ce9c37004 Prevent callback overruns in InvalidateBlock and RewindBlockIndex (Pieter Wuille)
9bb32eb571 Release cs_main during InvalidateBlock iterations (Pieter Wuille)
9b1ff5c742 Call InvalidateBlock without cs_main held (Pieter Wuille)
241b2c74ac Make RewindBlockIndex interruptible (Pieter Wuille)
880ce7d46b Call RewindBlockIndex without cs_main held (Pieter Wuille)
436f7d735f Release cs_main during RewindBlockIndex operation (Pieter Wuille)
1d342875c2 Merge the disconnection and erasing loops in RewindBlockIndex (Pieter Wuille)
32b2696ab4 Move erasure of non-active blocks to a separate loop in RewindBlockIndex (Pieter Wuille)
9d6dcc52c6 Abstract EraseBlockData out of RewindBlockIndex (Pieter Wuille)
Pull request description:
This PR makes a number of improvements to the InvalidateBlock (`invalidateblock` RPC) and RewindBlockIndex functions, primarily around breaking up their long-term cs_main holding. In addition:
* They're made safely interruptible (`bitcoind` can be shutdown, and no progress in either will be lost, though if incomplete, `invalidateblock` won't continue after restart and will need to be called again)
* The validation queue is prevented from overflowing (meaning `invalidateblock` on a very old block will not drive bitcoind OOM) (see #14289).
* `invalidateblock` won't bother to move transactions back into the mempool after 10 blocks (optimization).
This is not an optimal solution, as we're relying on the scheduler call sites to make sure the scheduler doesn't overflow. Ideally, the scheduler would guarantee this directly, but that needs a few further changes (moving the signal emissions out of cs_main) to prevent deadlocks.
I have manually tested the `invalidateblock` changes (including interrupting, and running with -checkblockindex and -checkmempool), but haven't tried the rewinding (which is probably becoming increasingly unnecessary, as very few pre-0.13.1 nodes remain that would care to upgrade).
Tree-SHA512: 692e42758bd3d3efc2eb701984a8cb5db25fbeee32e7575df0183a00d0c2c30fdf72ce64c7625c32ad8c8bdc56313da72a7471658faeb0d39eefe39c4b8b8474
faebd2ef40 doc: Move wallet lock annotations to header (MarcoFalke)
Pull request description:
We put the annotations in a central place (the header) as opposed to spreading them over the cpp files, where they easily get outdated.
Tree-SHA512: 18d8c7329efd3471713de18fe8d63d67c50fcb9fa99bc372294d829aa7668ea33e10d44e9e50121a04d8cc3302d5fd7759224f7935451a4693c4498a555257e6
fa38535130 bench: Benchmark MempoolToJSON (MarcoFalke)
fa5dc3534b rpc: Pass mempool into MempoolToJSON (MarcoFalke)
Pull request description:
This is used in production (e.g. https://jochen-hoenicke.de/queue/#0,24h), so add a benchmark to avoid making it even slower.
Related:
* "getrawmempool true RPC call is O(n^2)" #14765
Tree-SHA512: da09d2e54ee261af8671152f97f863cf1acd7a6adc6578e94046b1ec9e647a670c67499760ef765254f65522dfdf773c3c8729006fa2d63ccb6d53166bafc425
faa9b88199 fuzz: Link BasicTestingSetup (shared with unit tests) (MarcoFalke)
fa85468cd2 test: Move main_tests to validation_tests (MarcoFalke)
fa02b22245 test: Remove useless test_bitcoin_main.cpp (MarcoFalke)
fab2daa026 test: Add missing LIBBITCOIN_ZMQ to test_test_bitcoin_LDADD (MarcoFalke)
Pull request description:
Link against BasicTestingSetup in the fuzz tests, so we can fuzz against validation.
Also include a commit to remove test_bitcoin_main.cpp. That file may or may not overwrite globals in the link stage depending on the link order. This is confusing and useless anyway: The unit tests should never `std::exit` in the middle of the run (especially with success as exit code), since it will skip all test modules afterward.
Also include a commit to remove some unused forward declarations and move the main_tests to validation_tests, since main was long ago split into net_processing and validation.
Tree-SHA512: bdd34c87505450ec106d632f6664aadcbdac7c198172a77da55fab75b274f869ae1a8d06573ba2aff4cb186be9c7a34b7697894ab6f9c82b392f769c9135f36c
21be609b49 In lint-format-strings, open files sequentially (Glenn Willen)
Pull request description:
In lint-format-strings, we use python argparse to read our file arguments. In
this mode, argparse opens all the files simultaneously. On OS X, where the
default filehandle limit is 128, this causes the lint to fail. Instead, ask
argparse for our filename arguments as strings, and open them one at a time
using 'with open'.
Tree-SHA512: 4c7dabf98818a7c5d83ab10c61b89a26957fe399e39e933e30c561cb45c5e8ba6f6aedcde8343da0c32ee340289a8897db6a33708e35ee381334ee27e3f4d356
4d4e4c6448 Suggested interfaces::Chain cleanups from #15288 (Russell Yanofsky)
Pull request description:
Mostly documentation improvements requested in the last review of #15288 before it was merged (https://github.com/bitcoin/bitcoin/pull/15288#pullrequestreview-210241864)
Tree-SHA512: 64e912520bbec20a44032f265a8cf3f11ad7f5126c8626b5ad5e888227b1f92ecb321522fab4bbbd613230b55450abd6ace023631d0a4f357a780d65c5638bfe
fa2797808e test: Remove python3.4 workaround in feature_dbcrash (MarcoFalke)
dddd1d05d3 .python-version: Specify full version 3.5.6 (MarcoFalke)
faa7cdf764 scripted-diff: Update copyright in ./test (MarcoFalke)
fa0e65b772 scripted-diff: test: Remove brackets after assert (MarcoFalke)
fab5a1e0f4 build: Require python 3.5 (MarcoFalke)
fa6bf21f5e scripted-diff: test: Use py3.5 bytes::hex() method (MarcoFalke)
Pull request description:
Python 3.4 is EOL after March 2019, so switch to 3.5. See https://devguide.python.org/#status-of-python-branches
This pull does the following in a bunch of commits:
* scripted diff to use the `bytes::hex()` method in place of previous wrappers (`b2x`, `bytes_to_hex_str`, `hexlify`, ...)
* Update the build system (gitian and travis) to remove python2.7 and replace it with python3.5
* Another scripted-diff to remove brackets after `assert`. This is unrelated to the python3.5 switch, but a stylistic commit, so probably not worth to split up. The motivation behind it is to avoid asserting on data structures (such as tuples of length one), which never fails:
```py
>>> assert(False,) # with brackets
>>> assert False, # without brackets
SyntaxError: invalid syntax
>>> assert False # proper assertion
AssertionError
```
* And then a final scripted diff to update the copyright headers in the `test` subfolder, since I touched most of the files anyway and it wouldn't make sense to split this commit out into a separate pull.
For reference (contributed by luke-jr):
Ubuntu LTS (bionic): 3.6.5
Debian stable (stretch): 3.5.3
RHEL 8 (expected before v0.19): 3.6.x
Gentoo stable: 3.6.5
Arch: 3.7.1
Tree-SHA512: 643c28cd2d5b9543ce4bf8ad2a8b282bc79b37dc5b25c9c8358e6ce201e2a67a546463e5f3430b16652eb2489d7c3ed4b0772cd2e2bf790fe68a5e3cc8a25029
3eac2d57b1 docs: add "sections" info to example bitcoin.conf (Alistair Mann)
Pull request description:
Rebased / commit message fixed version of #15387.
This had ACKs, but just needed the commit message fixed up.
> Most bitcoin.conf options apply to all three networks,
however some apply only to mainnet unless specified in a section.
As stands, conf file has no indication that sections are now in use
or are in some circumstances mandatory (eg, changing rpcport for testnet.)
> Proposed change notifies the reader early which options are affected,
specifically adds those options affected but not already in the example,
adds brief explanation as to what's going on and provides a skeleton template for the sections themselves.
Tree-SHA512: 3331f2cac23d082bda2dcdea7d579360bc464d8e2123d634b810e9ba8edb5162bd62bd86f846b5299a04a3d77636a77e2fd3837c3272b22bc0d9a685d5156062
In lint-format-strings, we use python argparse to read our file arguments. In
this mode, argparse opens all the files simultaneously. On OS X, where the
default filehandle limit is 128, this causes the lint to fail. Instead, ask
argparse for our filename arguments as strings, and open them one at a time
using 'with open'.
fa45123f66 test: Add .style.yapf (MarcoFalke)
Pull request description:
This can *optionally* be used to format any added code before submitting a pull. I use this heavily and wouldn't want to hold it back from others, now that yapf is referred to in https://github.com/bitcoin/bitcoin/blob/master/doc/productivity.md#format-python-diffs-with-yapf-diffpy
Tree-SHA512: 0f3d8bcbb76a710d9faa1226202073e8d967a82a05fc002cd10305ff58b382f5ff3df96a6faaec5bd01613d41f5fc2343e4999fb1217bf1f24f6da186d572ca1
f7efd87c8f Change brace formatting (Russell Yanofsky)
a1df1b48a8 Remove use of IsInitialBlockDownload in wallet code (Russell Yanofsky)
1106a6fde4 Remove use of uiInterface.LoadWallet in wallet code (Russell Yanofsky)
318f41fb2c circular-dependencies: Avoid treating some .h/.cpp files as a unit (Russell Yanofsky)
d02b34c8a8 Remove use of AcceptToMemoryPool in wallet code (Russell Yanofsky)
e2c8ba9f6e Remove uses of InitMessage/Warning/Error in wallet code (Russell Yanofsky)
c5e59a96a8 Remove uses of GetAdjustedTime in wallet code (Russell Yanofsky)
6d6bcc77c0 Remove use of g_connman / PushInventory in wallet code (Russell Yanofsky)
00dfb2a440 Remove uses of g_connman in wallet code (Russell Yanofsky)
cc3836e8f9 Remove uses of fPruneMode in wallet code (Russell Yanofsky)
cc02c796d3 Remove uses of fee globals in wallet code (Russell Yanofsky)
1fb0a4a04e Remove use of CalculateMemPoolAncestors in wallet code (Russell Yanofsky)
cd32160af0 Remove use of GetTransactionAncestry in wallet code (Russell Yanofsky)
291276f7f4 Remove use of GetCountWithDescendants in wallet code (Russell Yanofsky)
bdc6628683 Remove use of IsRBFOptIn in wallet code (Russell Yanofsky)
80f52a2267 Remove uses of CheckFinalTx in wallet code (Russell Yanofsky)
Pull request description:
This change removes wallet calls to node functions that access global chain and mempool state.
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: 40dbaf1f59fb22b32e70b054b30ba5638d638aa3240fa30e0f721d53c721cd6138a7ab4d423a24d7d2fda0b956e68d44c733abc2c9259c3d6c9fd6d4be89aa23
fa3148aacb doc: Remove ppa from linux build instructions (MarcoFalke)
Pull request description:
bdb (5.x) is provided by the vanilla Debian/Ubuntu package manager.
If someone needs bdb4.8, they can use `./depends` or `./contrib/install_db4.sh`.
I don't think we need a forth way to install bdb. Those ppa instructions only leads to confusion for debian users: Closes#3757
Tree-SHA512: 54620c28dfd63f0e30f1b0f4f4f4dcb66a5bbdd3ec60621860b005eee0a70fc6a5f3babdc5719ba5d3abda7fefc720eb1d8c70633b8a20a269ebc46e6206c781
04cca33094 Style cleanup. (Jim Posen)
4c01e4e159 flatfile: Unit tests for FlatFileSeq methods. (Jim Posen)
65a489e93d scripted-diff: Rename CBlockDiskPos to FlatFilePos. (Jim Posen)
d6d8a78f26 Move CDiskBlockPos from chain to flatfile. (Jim Posen)
e0380933e3 validation: Refactor file flush logic into FlatFileSeq. (Jim Posen)
992404b31e validation: Refactor block file pre-allocation into FlatFileSeq. (Jim Posen)
e2d2abb99f validation: Refactor OpenDiskFile into method on FlatFileSeq. (Jim Posen)
9183d6ef65 validation: Extract basic block file logic into FlatFileSeq class. (Jim Posen)
62e7addb63 util: Move CheckDiskSpace to util. (Jim Posen)
Pull request description:
This cleans up and refactors block file helpers so that they may be used by the block filter indexer. Per [design discussion](https://github.com/bitcoin/bitcoin/pull/14121#issuecomment-451252591) about storing BIP 157 block filters, it has been suggested that they are stored in the same way as block and undo data. This refactor is sufficient to simplify file operations for this use case, though in the future perhaps more pruning-related logic ought to be moved into the new classes.
The basic abstraction is a `FlatFileSeq` which manages access to a sequence of numbered files into which raw data is written.
Tree-SHA512: b2108756777f2dad8964a1a2ef2764486e708a4a4a8cfac47b5de8bcb0625388964438eb096b10cfd9ea39212c299b5cb32fa943e768db2333cf49ea7def157e
1a7ba84e11 Fix lack of warning of unrecognized section names (Akio Nakamura)
Pull request description:
In #14708, It was introduced that to warn when unrecognized section names are exist in the config file.
But ```m_config_sections.clear()``` in ```ArgsManager::ReadConfigStream()``` is called every time when reading each configuration file, so it can warn about only last reading file if ```includeconf``` exists.
This PR fix lack of warning by collecting all section names by moving ```m_config_sections.clear()``` to ```ArgsManager::ReadConfigFiles()``` .
Also add a test code to confirm this situation.
Tree-SHA512: 26aa0cbe3e4ae2e58cbe73d4492ee5cf465fd4c3e5df2c8ca7e282b627df9e637267af1e3816386b1dc6db2398b31936925ce0e432219fec3a9b3398f01e3e65
43206239a8 tests: Add script checking for deterministic line coverage (practicalswift)
Pull request description:
Add script checking for deterministic line coverage in unit tests.
Context: #14343 ("coverage reports non-deterministic")
When the coverage is deterministic this script can be invoked from Travis to guard against regressions, but left inactive for now.
Output in case of determinism:
```
$ contrib/test_deterministic_coverage.sh 2
[2019-01-30 20:08:46] Measuring coverage, run #1 of 2
[2019-01-30 20:10:45] Measuring coverage, run #2 of 2
Coverage test passed: Deterministic coverage across 2 runs.
```
Output in case of non-determinism:
```
$ contrib/test_deterministic_coverage.sh 2
[2019-01-30 20:08:46] Measuring coverage, run #1 of 2
[2019-01-30 20:10:45] Measuring coverage, run #2 of 2
The line coverage is non-deterministic between runs.
The test suite must be deterministic in the sense that the set of lines executed at least
once must be identical between runs. This is a neccessary condition for meaningful coverage
measuring.
--- gcovr.run-1.txt 2019-01-30 23:14:07.419418694 +0100
+++ gcovr.run-2.txt 2019-01-30 23:15:57.998811282 +0100
@@ -471,7 +471,7 @@
test/crypto_tests.cpp 270 270 100%
test/cuckoocache_tests.cpp 142 142 100%
test/dbwrapper_tests.cpp 148 148 100%
-test/denialofservice_tests.cpp 225 225 100%
+test/denialofservice_tests.cpp 225 224 99% 363
test/descriptor_tests.cpp 116 116 100%
test/fs_tests.cpp 24 3 12% 14,16-17,19-20,23,25-26,29,31-32,35-36,39,41-42,45-46,49,51-52
test/getarg_tests.cpp 111 111 100%
@@ -585,5 +585,5 @@
zmq/zmqpublishnotifier.h 5 0 0% 12,31,37,43,49
zmq/zmqrpc.cpp 21 0 0% 16,18,20,22,33-35,38-45,49,52,56,60,62-63
------------------------------------------------------------------------------
-TOTAL 61561 27606 44%
+TOTAL 61561 27605 44%
------------------------------------------------------------------------------
```
In this case line 363 of `test/denialofservice_tests.cpp` was executed only in the second run. Non-determinism detected!
Tree-SHA512: 03f45590e70a87146f89aa7838beeff0925d7fd303697ff03e0e69f8a5861694be5f0dd10cb0020e3e3d40c9cf662f71dfcd838f6affb31bd5212314e0a4e3a9
07cae5287c [wallet] remove unused GetScriptForMining (Sjors Provoost)
8bb3e4c487 [rpc] remove deprecated generate method (Sjors Provoost)
Pull request description:
As announced in v0.18, the wallet generate rpc method is deprecated and will be fully removed in v0.19.
Clients should transition to using the node rpc method `generatetoaddress`.
Tree-SHA512: 9e5e913b59f3e18440b2b7b356124c7b87ad19f81a1ab6ada06a6c396b84e734895465f569296f1ba8c12abf74863bab5fd77765c9e806c239713aa83a59485f
