aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2020-08-07Merge #19098: test: Remove duplicate NodeContext hacksMarcoFalke
edc316020e8270dafc5e31465d532baebdafd3dd test: Remove duplicate NodeContext hacks (Russell Yanofsky) Pull request description: Qt tests currently are currently using two NodeContext structs at the same time, one in interfaces::NodeImpl::m_context, and the other in BasicTestingSetup::m_node, and the tests have hacks transferring state between them. Fix this by getting rid of the NodeImpl::m_context struct and making it a pointer. This way a common BitcoinApplication object can be used for all qt tests, but they can still have their own testing setups. Non-test code is changing but non-test behavior is still the same as before. Motivation for this PR is to be able to remove the "std::move(test.m_node.connman)" and mempool hacks for swapping individual NodeContext members in Qt tests, because followup PR #19099 adds yet another member (wallet_client) that needs to be swapped. After this change, the whole NodeContext struct can be swapped instead of individual members, so the workarounds are less fragile and invasive. ACKs for top commit: MarcoFalke: crACK edc316020e8270dafc5e31465d532baebdafd3dd 🌮 promag: ACK edc316020e8270dafc5e31465d532baebdafd3dd. Tree-SHA512: c1650e4127f43a4020304ca7c13b5d9122fb5723aacd8fa1cf855d03c6052fcfb7685810aa2a5ef708561015f0022fecaacbad479295104ca45d2c17579466a4
2020-08-05Merge #15382: util: add RunCommandParseJSONSamuel Dobson
31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0 [util] add RunCommandParseJSON (Sjors Provoost) c17f54ee535faaedf9033717403e1f775b5f1530 [ci] use boost::process (Sjors Provoost) 32128ba682033560d6eb2e4848a9f77a842016d2 [doc] include Doxygen comments for HAVE_BOOST_PROCESS (Sjors Provoost) 3c84d85f7d218fa27e9343c5cd1a55e519218980 [build] msvc: add boost::process (Sjors Provoost) c47e4bbf0b44f2de1278f9538124ec98ee0815bb [build] make boost-process opt-in (Sjors Provoost) 929cda5470f98d1ef85c05b1cad4e2fb9227e3b0 configure: add ax_boost_process (Sjors Provoost) 8314c23d7b39fc36dde8b40b03b6efbe96f85698 [depends] boost: patch unused variable in boost_process (Sjors Provoost) Pull request description: Prerequisite for external signer support in #16546. Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d). This adds a new dependency [boost process](https://github.com/boostorg/process/tree/boost-1.64.0). This is part of Boost since 1.64 which is part of `depends`. Because the minimum Boost version is 1.47, this functionality is skipped for older versions of Boost. Use `./configure --with-boost-process` to opt in, which checks for the presence of Boost::Process. We add `UniValue runCommandParseJSON(const std::string& strCommand)` to `system.{h,cpp}` which calls an arbitrary command and processes the JSON returned by it. This is currently only called by the test suite. ~For testing purposes this adds a new regtest-only RPC method `runcommand`, as well as `test/mocks/command.py` used by functional tests.~ (this is no longer the case) TODO: - [ ] review boost process in #15440 ACKs for top commit: achow101: ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0 hebasto: re-ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, only rebased (verified with `git range-diff`) and removed an unintentional tab character since the [previous](https://github.com/bitcoin/bitcoin/pull/15382#pullrequestreview-458371035) review. meshcollider: Very light utACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, although I am not very confident with build stuff. promag: Code review ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, don't mind the nit. ryanofsky: Code review ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0. I left some comments below that could be ignored or followed up later. The current change is clean and comprehensive. Tree-SHA512: c506e747014b263606e1f538ed4624a8ad7bcf4e025cb700c12cc5739964e254dc04a2bbb848996b170e2ccec3fbfa4fe9e2b3976b191222cfb82fc3e6ab182d
2020-08-04Merge #19340: Preserve the `LockData` initial state if "potential deadlock ↵MarcoFalke
detected" exception thrown 63e9e40b73507b0c9361fc8728d4e97fd72c9ec9 test: Add LockStackEmpty() (Hennadii Stepanov) 42b2a953735457cbf7b62a18b89811a66e573298 test: Repeat deadlock tests (Hennadii Stepanov) 1f96be25b020a56afa330286ee4f241aa14d3983 Preserve initial state if push_lock() throws exception (Hennadii Stepanov) Pull request description: On master (e3fa3c7d671e34038a262bb2db16b30bee82153d) if the `push_lock()` throws the "potential deadlock detected" exception (via the `potential_deadlock_detected()` call), the `LockData` instance internal state differs from one when the `push_lock()` was called. This non-well behaviour makes (at least) testing brittle. This PR preserves the `LockData` instance initial state if `push_lock()` throws an exception, and improves the `sync_tests` unit test. ACKs for top commit: MarcoFalke: re-ACK 63e9e40b73 vasild: ACK 63e9e40 Tree-SHA512: 7679182154ce5f079b44b790faf76eb5f553328dea70a326ff6b600db70e2f9ae015a33a104ca070cb660318280cb79b6b42e37ea5166f26f9e627ba721fcdec
2020-08-03Merge #19326: Simplify hash.h interface using SpansWladimir J. van der Laan
77c507358bda9bd6c496f33e0f4418c0603bb08d Make Hash[160] consume range-like objects (Pieter Wuille) 02c4cc5c5ddf61f98ee366a4bea8abc26de492bd Make CHash256/CHash160 output to Span (Pieter Wuille) 0ef97b1b103231db54e04a64bbdb5dcc3f34f482 Make MurmurHash3 consume Spans (Pieter Wuille) e549bf8a9afae42fcda805e216a1cde62df195a6 Make CHash256 and CHash160 consume Spans (Pieter Wuille) 2a2182c387f607cd8284f33890bd285a81077b7f Make script/standard's BaseHash Span-convertible (Pieter Wuille) e63dcc3a6752e7d406e7a650c2d6c2e95cd39aab Add MakeUCharSpan, to help constructing Span<[const] unsigned char> (Pieter Wuille) 567825049fb0e47e698dcaad9caa65693a6b42d3 Make uint256 Span-convertible by adding ::data() (Pieter Wuille) 131a2f0337f5c396739a47b60bb856ed84ec8937 scripted-diff: rename base_blob::data to m_data (Pieter Wuille) Pull request description: This makes use of the implicit constructions and conversions to Span introduced in #18468 to simplify the hash.h interface: * All functions that take a pointer and a length are changed to take a Span instead. * The Hash() and Hash160() functions are changed to take in "range" objects instead of begin/end iterators. ACKs for top commit: laanwj: re-ACK 77c507358bda9bd6c496f33e0f4418c0603bb08d jonatack: Code review re-ACK 77c5073 per `git range-diff 14ceddd 49fc016 77c5073` Tree-SHA512: 9ec929891b1ddcf30eb14b946ee1bf142eca1442b9de0067ad6a3c181e0c7ea0c99c0e291e7f6e7a18bd7bdf78fe94ee3d5de66e167401674caf91e026269771
2020-08-03Merge #18991: Cache responses to GETADDR to prevent topology leaksWladimir J. van der Laan
3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6 Test addr response caching (Gleb Naumenko) cf1569e074505dbbb9d29422803dd31bb62072d4 Add addr permission flag enabling non-cached addr sharing (Gleb Naumenko) acd6135b43941fa51d52f5fcdb2ce944280ad01e Cache responses to addr requests (Gleb Naumenko) 7cc0e8101f01891aa8be093a00d993bb7579c385 Remove useless 2500 limit on AddrMan queries (Gleb Naumenko) ded742bc5b96e3215d69c11fb3628d224e7ae034 Move filtering banned addrs inside GetAddresses() (Gleb Naumenko) Pull request description: This is a very simple code change with a big p2p privacy benefit. It’s currently trivial to scrape any reachable node’s AddrMan (a database of all nodes known to them along with the timestamps). We do have a limit of one GETADDR per connection, but a spy can disconnect and reconnect even from the same IP, and send GETADDR again and again. Since we respond with 1,000 random records at most, depending on the AddrMan size it takes probably up to 100 requests for an spy to make sure they scraped (almost) everything. I even have a script for that. It is totally doable within couple minutes. Then, with some extra protocol knowledge a spy can infer the direct peers of the victim, and other topological stuff. I suggest to cache responses to GETADDR on a daily basis, so that an attacker gets at most 1,000 records per day, and can’t track the changes in real time. I will be following up with more improvements to addr relay privacy, but this one alone is a very effective. And simple! I doubt any of the real software does *reconnect to get new addrs from a given peer*, so we shouldn’t be cutting anyone. I also believe it doesn’t have any negative implications on the overall topology quality. And the records being “outdated” for at most a day doesn’t break any honest assumptions either. ACKs for top commit: jnewbery: reACK 3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6 promag: Code review ACK 3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6. ariard: Code Review ACK 3bd67ba Tree-SHA512: dfa5d03205c2424e40a3f8a41af9306227e1ca18beead3b3dda44aa2a082175bb1c6d929dbc7ea8e48e01aed0d50f0d54491caa1147471a2b72a46c3ca06b66f
2020-08-02test: Add LockStackEmpty()Hennadii Stepanov
2020-08-02test: Repeat deadlock testsHennadii Stepanov
2020-07-31[util] add RunCommandParseJSONSjors Provoost
2020-07-30Make Hash[160] consume range-like objectsPieter Wuille
2020-07-30Make CHash256/CHash160 output to SpanPieter Wuille
2020-07-30Make CHash256 and CHash160 consume SpansPieter Wuille
2020-07-30Merge #19604: Pass mempool pointer to UnloadBlockIndex/GetCoinsCacheSizeStateMarcoFalke
fae8c28dae747f9c4c6481049742346d18202fc8 Pass mempool pointer to GetCoinsCacheSizeState (MarcoFalke) fac674db200e6b2d5b32069335fb24e713d7b69f Pass mempool pointer to UnloadBlockIndex (MarcoFalke) faec851b6eb7e65e28cdcae50d6dc86fafa0f91c test: Simplify cs_main locks (MarcoFalke) Pull request description: Split out from #19556 Instead of relying on the implicit mempool global, pass a mempool pointer (which can be `0`). This helps with testing, code clarity and unlocks the features described in #19556. ACKs for top commit: jnewbery: code review ACK fae8c28dae747f9c4c6481049742346d18202fc8 fjahr: Code review ACK fae8c28dae747f9c4c6481049742346d18202fc8 darosior: Tested ACK fae8c28dae747f9c4c6481049742346d18202fc8 jamesob: ACK fae8c28dae747f9c4c6481049742346d18202fc8 ([`jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to`](https://github.com/jamesob/bitcoin/tree/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to)) Tree-SHA512: fa687518c8cda4a095bdbdfe56e01fae2fb16c13d51efbb1312cd6dc007611fc47f53f475602e4a843e3973c9410e6af5a81d6847bd2399f8262ca7205975728
2020-07-30Add addr permission flag enabling non-cached addr sharingGleb Naumenko
2020-07-29Merge #19534: net: save the network type explicitly in CNetAddrWladimir J. van der Laan
bcfebb6d5511ad4c156868bc799831ace628a225 net: save the network type explicitly in CNetAddr (Vasil Dimov) 100c64a95b518a6a19241aec4058b866a8872d9b net: document `enum Network` (Vasil Dimov) Pull request description: (chopped off from https://github.com/bitcoin/bitcoin/pull/19031 to ease review) Before this change, we would analyze the contents of `CNetAddr::ip[16]` in order to tell which type is an address. Change this by introducing a new member `CNetAddr::m_net` that explicitly tells the type of the address. This is necessary because in BIP155 we will not be able to tell the address type by just looking at its raw representation (e.g. both TORv3 and I2P are "seemingly random" 32 bytes). As a side effect of this change we no longer need to store IPv4 addresses encoded as IPv6 addresses - we can store them in proper 4 bytes (will be done in a separate commit). Also the code gets somewhat simplified - instead of `memcmp(ip, pchIPv4, sizeof(pchIPv4)) == 0` we can use `m_net == NET_IPV4`. ACKs for top commit: troygiorshev: reACK bcfebb6d5511ad4c156868bc799831ace628a225 via `git range-diff master 64897c5 bcfebb6` jonatack: re-ACK bcfebb6 per `git diff 662bb25 bcfebb6`, code review, debug build/tests clean, ran bitcoind. laanwj: Code review ACK bcfebb6d5511ad4c156868bc799831ace628a225 Tree-SHA512: 9347e2a50feac617a994bfb46a8f77e31c236bde882e4fd4f03eea4766cd5110216f5f3d24dee91d25218bab7f8bb6e1d2d6212a44db9e34594299fd6ff7606b
2020-07-29Pass mempool pointer to GetCoinsCacheSizeStateMarcoFalke
2020-07-29Pass mempool pointer to UnloadBlockIndexMarcoFalke
2020-07-29test: Simplify cs_main locksMarcoFalke
2020-07-29Merge #18637: coins: allow cache resize after initMarcoFalke
f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4 test: add test for CChainState::ResizeCoinsCaches() (James O'Beirne) 8ac3ef46999ed676ca3775f7b2f461d92f09a542 add ChainstateManager::MaybeRebalanceCaches() (James O'Beirne) f36aaa6392fdbdac6891d92202d3efeff98754f4 Add CChainState::ResizeCoinsCaches (James O'Beirne) b223111da2e0e9ceccef75df8a20252b0094b7bc txdb: add CCoinsViewDB::ChangeCacheSize (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- In the assumeutxo implementation draft (#15056), once a UTXO snapshot is loaded, a new chainstate object is created after initialization. This means that we have to reclaim some of the cache that we've allocated to the original chainstate (per `dbcache=`) to repurpose for the snapshot chainstate. Furthermore, it makes sense to have different cache allocations depending on which chainstate is more active. While the snapshot chainstate is working to get to the network tip (and the background validation chainstate is idle), it makes sense that the snapshot chainstate should have the majority of cache allocation. And contrariwise once the snapshot has reached network tip, most of the cache should be given to the background validation chainstate. This set of changes (detailed in the commit messages) allows us to dynamically resize the various coins caches. None of the functionality introduced here is used at the moment, but will be in the next AU PR (which introduces `ActivateSnapshot`). `ChainstateManager::MaybeRebalanceCaches()` defines the (somewhat normative) cache allocations between the snapshot and background validation chainstates. I'd be interested in feedback if anyone has thoughts on the proportions I've set there. ACKs for top commit: ajtowns: weak utACK f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4 -- didn't find any major problems, but not super confident that I didn't miss anything fjahr: Code review ACK f19fdd4 ryanofsky: Code review ACK f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4. Only change since last review is constructor cleanup (no change in behavior). I think the suggestions here from ajtowns and others are good, but shouldn't delay merging the PR (and hold up assumeutxo) Tree-SHA512: fffb7847fb6993dd4a1a41cf11179b211b0b20b7eb5f7cf6266442136bfe9d43b830bbefcafd475bfd4af273f5573500594aa41fff03e0ed5c2a1e8562ff9269
2020-07-27net: save the network type explicitly in CNetAddrVasil Dimov
Before this change, we would analyze the contents of `CNetAddr::ip[16]` in order to tell which type is an address. Change this by introducing a new member `CNetAddr::m_net` that explicitly tells the type of the address. This is necessary because in BIP155 we will not be able to tell the address type by just looking at its raw representation (e.g. both TORv3 and I2P are "seemingly random" 32 bytes). As a side effect of this change we no longer need to store IPv4 addresses encoded as IPv6 addresses - we can store them in proper 4 bytes (will be done in a separate commit). Also the code gets somewhat simplified - instead of `memcmp(ip, pchIPv4, sizeof(pchIPv4)) == 0` we can use `m_net == NET_IPV4`. Co-authored-by: Carl Dong <contact@carldong.me>
2020-07-26refactor: Drop unused CBufferedFile::Seek()Hennadii Stepanov
2020-07-23Merge #15935: Add <datadir>/settings.json persistent settings storageMarcoFalke
9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8 Add <datadir>/settings.json persistent settings storage. (Russell Yanofsky) eb682c5700e7a9176d0104d470b83ff9aa3589e8 util: Add ReadSettings and WriteSettings functions (Russell Yanofsky) Pull request description: Persistent settings are used in followup PRs #15936 to unify gui settings between bitcoin-qt and bitcoind, and #15937 to add a load_on_startup flag to the loadwallet RPC and maintain a dynamic list of wallets that should be loaded on startup that also can be shared between bitcoind and bitcoin-qt. ACKs for top commit: MarcoFalke: Approach re-ACK 9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8 🌾 jnewbery: utACK 9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8 Tree-SHA512: 39fcc6051717117c9141e934de1d0d3f739484be4685cdf97d54de967c8c816502b4fd0de12114433beaa5c5b7060c810fd8ae4e2b3ce7c371eb729ac01ba2e1
2020-07-22Merge #19562: test: Fix fuzzer compilation on macOSfanquake
c8992e89594a54edf283e4916f794475070b5114 test: Fix fuzzer compilation on macOS fixes #19557 (freenancial) Pull request description: fixes #19557 Before the fix: ``` âžś bitcoin git:(fix-fuzzer-macos) make Making all in src CXX test/fuzz/addition_overflow-addition_overflow.o In file included from test/fuzz/addition_overflow.cpp:7: ./test/fuzz/util.h:335:13: error: no matching function for call to 'AdditionOverflow' if (AdditionOverflow((uint64_t)fuzzed_file->m_offset, random_bytes.size())) { ^~~~~~~~~~~~~~~~ ./test/fuzz/util.h:201:16: note: candidate template ignored: deduced conflicting types for parameter 'T' ('unsigned long long' vs. 'unsigned long') NODISCARD bool AdditionOverflow(const T i, const T j) noexcept ^ ./test/fuzz/util.h:346:13: error: no matching function for call to 'AdditionOverflow' if (AdditionOverflow(fuzzed_file->m_offset, n)) { ^~~~~~~~~~~~~~~~ ./test/fuzz/util.h:201:16: note: candidate template ignored: deduced conflicting types for parameter 'T' ('long long' vs. 'long') NODISCARD bool AdditionOverflow(const T i, const T j) noexcept ^ ``` After the fix: ``` âžś bitcoin git:(fix-fuzzer-macos) ./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm && make clean && make -j5 ... ... CXXLD test/fuzz/uint256_deserialize Making all in doc/man make[1]: Nothing to be done for `all'. make[1]: Nothing to be done for `all-am'. ``` ACKs for top commit: fanquake: ACK c8992e89594a54edf283e4916f794475070b5114 - tested that compiling works on macOS. MarcoFalke: review ACK c8992e89594a54edf283e4916f794475070b5114 Tree-SHA512: 965cdc61b30db0e2209c91b29f0d42de927a9a5b85e1e70f22d1452e0955f876726c7a8c1d1a5f448f12bf24eec3000802071cd4ae28d8605343fd43d174ca84
2020-07-22Merge #19548: fuzz: add missing overrides to signature_checkerfanquake
c0f09c2c9deaec4cfb35ea587363e6301dd17b88 fuzz: add missing overrides to signature_checker (Jon Atack) Pull request description: These functions in `fuzz/signature_checker.cpp` override virtual member functions and should be marked `override` instead of `virtual`, which is for introducing a new virtual function. The overridden virtual functions are in `script/interpreter.h:151/156/161`. Also, per MarcoFalke suggestion, add missing parentheses in `fuzz/scriptnum_ops.cpp` and remove useless `unsigned int >= 0` conditional in `fuzz/script.cpp`. These changes fix 5 compile warnings in gcc 10 and 3 in clang 11/12. ACKs for top commit: vasild: ACK c0f09c2 MarcoFalke: review ACK c0f09c2c9deaec4cfb35ea587363e6301dd17b88 Tree-SHA512: 76ce73ec577c1f23cf8646c31d44dcd6c6303732c47187d041a8921d0d24a50163989a375352ebc221abf2ac337bc0902149be46b6f9eebc071d2f364c407f71
2020-07-22fuzz: add missing overrides to signature_checkerJon Atack
and also - add missing parentheses in fuzz/scriptnum_ops.cpp - remove useless unsigned int conditional in fuzz/script.cpp These changes fix 5 compile warnings in gcc 10.
2020-07-21test: Fix fuzzer compilation on macOSfreenancial
fixes #19557
2020-07-21Merge #16878: Fix non-deterministic coverage of test DoS_mapOrphansMarcoFalke
4455949d6f0218b40d33d7fe6de6555f8f62192f Make test DoS_mapOrphans deterministic (David Reikher) Pull request description: This pull request proposes a solution to make the test `DoS_mapOrphans` in denialofservice_tests.cpp have deterministic coverage. The `RandomOrphan` function in denialofservice_tests.cpp and the implicitly called function `ecdsa_signature_parse_der_lax` in pubkey.cpp were causing the non-deterministic test coverage. In the former, if a random orphan was selected the index of which is bigger than the max. orphan index in `mapOrphanTransactions`, the last orphan was returned from `RandomOrphan`. If the random number generated was never large enough, this condition would not be fulfilled and the corresponding branch wouldn't run. The proposed solution is to force one of the 50 dependant orphans to depend on the last orphan in `mapOrphanTransactions` using the newly introduced function `OrphanByIndex` (and passing it a large uint256), forcing this branch to run at least once. In the latter, if values for ECDSA `R` or `S` (or both) had no leading zeros, some code would not be executed. The solution was to find a constant signature that would be comprised of `R` and `S` values with leading zeros and calling `CPubKey::Verify` at the end of the test with this signature forcing this code to always run at least once at the end even if it hadn't throughout the test. To test that the coverage is (at least highly likely) deterministic, I ran `contrib/devtools/test_deterministic_coverage.sh denialofservice_tests/DoS_mapOrphans 1000` and the result was deterministic coverage across 1000 runs. Also - removed denialofservice_tests test entry from the list of non-deterministic tests in the coverage script. ACKs for top commit: MarcoFalke: ACK 4455949d6f0218b40d33d7fe6de6555f8f62192f Tree-SHA512: 987eb1f94b80d5bec4d4944e91ef43b9b8603055750362d4b4665b7f011be27045808aa9f4c6ccf8ae009b61405f9a1b8671d65a843c3328e5b8acce1f1c00a6
2020-07-21Make test DoS_mapOrphans deterministicDavid Reikher
The RandomOrphan function and the function ecdsa_signature_parse_der_lax in pubkey.cpp were causing non-deterministic test coverage. Force seed in the beginning of the test to make it deterministic. The seed is selected carefully so that all branches of the function ecdsa_signature_parse_der_lax are executed. Prior to this fix, the test was exhibiting non-deterministic coverage since none of the ECDSA signatures that were generated during the test had leading zeroes in either R, S, or both, resulting in some branches of said function not being executed. The seed ensures that both conditions are hit. Removed denialofservice_tests test entry from the list of non-deterministic tests in the coverage script.
2020-07-19fuzz: fix unused variable addrdb compiler warningJon Atack
2020-07-18Merge #19143: tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, ↵MarcoFalke
LoadExternalBlockFile and other FILE* consumers ad6c34881dc125c973b6b9ba1daa999d3141b1ae tests: Add fuzzing harness for CBlockPolicyEstimator::{Read,Write} (policy/fees.h) (practicalswift) 614e0807a8137d82832aea45e4864b424f71f698 tests: Add fuzzing harness for CBufferedFile::{SetPos,GetPos,GetType,GetVersion} (stream.h) (practicalswift) 7bcc71e5f8cdfd8ba1411c799c0726f503e52343 tests: Add fuzzing harness for LoadExternalBlockFile(...) (validation.h) (practicalswift) 98233760305a36acbd41d76aeebeada1340f6367 tests: Add fuzzing harness for CBufferedFile (streams.h) (practicalswift) f3aa659be676a4dd0c20fe6c5cb4acd7a5b38b76 tests: Add fuzzing harness for CAutoFile (streams.h) (practicalswift) e507c0799d759355dd0cfbe83449f0f767a7264e tests: Add serialization/deserialization fuzzing helpers WriteToStream(…)/ReadFromStream(…) (practicalswift) e48094a506ad031d211b9dfe7639d8b3a2239788 tests: Add FuzzedAutoFileProvider which provides a CAutoFile interface to FuzzedDataProvider (practicalswift) 9dbcd6854ca05a9bd1e9a5e1222dac1758048231 tests: Add FuzzedFileProvider which provides a FILE* interface to FuzzedDataProvider using fopencookie (practicalswift) Pull request description: Add fuzzing harnesses for `CAutoFile`, `CBufferedFile`, `LoadExternalBlockFile` and other `FILE*` consumers: * Add `FuzzedFileProvider` which provides a `FILE*` interface to `FuzzedDataProvider` using `fopencookie` * Add `FuzzedAutoFileProvider` which provides a `CAutoFile` interface to `FuzzedDataProvider` * Add serialization/deserialization fuzzing helpers `WriteToStream(…)`/`ReadFromStream(…)` * Add fuzzing harness for `CAutoFile` (`streams.h`) * Add fuzzing harness for `CBufferedFile` (`streams.h`) * Add fuzzing harness for `LoadExternalBlockFile(...)` (`validation.h`) * Add fuzzing harness for `CBlockPolicyEstimator::Read` and `CBlockPolicyEstimator::Write` (`policy/fees.h`) See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) ACKs for top commit: Crypt-iQ: Tested ACK ad6c348 Tree-SHA512: a38e142608218496796a527d7e59b74e30279a2815450408b7c27a76ed600cebc6b88491e831665a0639671e2d212453fcdca558500bbadbeb32b267751f8f72
2020-07-16Merge #19533: [tests] Remove unnecessary cs_mains in denialofservice_testsMarcoFalke
f58c4b538ebd67fcfea0a4aff5e062fd59fb19f5 [tests] Remove unnecessary cs_mains in denialofservice_tests (Matt Corallo) Pull request description: 9fdf05d70cac4a62d1aeeb4299e2c3a9a866f8af resolved some lock inversion warnings in denialofservice_tests, but left in a number of cs_main locks that are unnecessary (introducing lock inversion warnings in future changes). ACKs for top commit: promag: ACK f58c4b538ebd67fcfea0a4aff5e062fd59fb19f5. jonatack: ACK f58c4b538ebd67fcfe verified the test locks correspond to the locks in net/net_processing, and the debug build is clean/unit tests pass. Tree-SHA512: de2d9b2a8f08081b2ce31e18585e4677b167a11752b797d790c281575d7dfef3587f8be4fc7f8f16771141b6ff0b0145c7488cf30e79256b0043947c67a6182c
2020-07-16Merge #19174: refactor: replace CConnman pointers by references in ↵MarcoFalke
net_processing.cpp 0c8461a88ed66a1f70559fc96646708949b17e4b refactor: replace CConnman pointers by references in net_processing.cpp (Sebastian Falbesoner) Pull request description: This is a follow-up to the recently merged PR https://github.com/bitcoin/bitcoin/pull/19053, replacing ~~two more types of~~ one more type of pointer (CConnman) by references to increase the code quality -- pointers should either check for `nullptr` or be replaced by references, and the latter strategy seems to be more reasonable. Again, to keep the review burden managable, the changes are kept simple, * only tackling `CConnman*` ~~and `BanMan*`~~ pointers * only within the net_processing module, i.e. no changes that would need adaption in other modules * keeping the names of the variables as they are ACKs for top commit: jnewbery: utACK 0c8461a88ed66a1f70559fc96646708949b17e4b MarcoFalke: ACK 0c8461a88ed66a1f70559fc96646708949b17e4b 🕧 Tree-SHA512: 79dc05144bcfb5e0bbc62180285aadcc6199f044fa3016c0f54f7b7f45037415260970037bd63b18fafefb8aef448549dae14b780bafb540fa2373f493a17f71
2020-07-15Merge #19353: Fix mistakenly swapped "previous" and "current" lock ordersWladimir J. van der Laan
0ecff9dd3418e8c18fa423ba53e9cab1df8be553 Improve "detected inconsistent lock order" error message (Hennadii Stepanov) bbe9cf4fe4ff9a8d1ea557fb763c76100db07679 test: Improve "potential deadlock detected" exception message (Hennadii Stepanov) 35599344c886b62f198e35fd940c1ab15c4a9f90 Fix mistakenly swapped "previous" and "current" lock orders (Hennadii Stepanov) Pull request description: In master (8ef15e8a86038225afef2487ca23abc10ca5dffa) the "previous" and "current" lock orders are mistakenly swapped. This PR: - fixes printed lock orders - improves the `sync_tests` unit test - makes the "detected inconsistent lock order" error message pointing to the lock location rather `tfm::format()` location. Debugger output example with this PR (with modified code, of course): ``` 2020-06-22T15:46:56Z [msghand] POTENTIAL DEADLOCK DETECTED 2020-06-22T15:46:56Z [msghand] Previous lock order was: 2020-06-22T15:46:56Z [msghand] (2) 'cs_main' in net_processing.cpp:2545 (in thread 'msghand') 2020-06-22T15:46:56Z [msghand] (1) 'g_cs_orphans' in net_processing.cpp:1400 (in thread 'msghand') 2020-06-22T15:46:56Z [msghand] Current lock order is: 2020-06-22T15:46:56Z [msghand] (1) 'g_cs_orphans' in net_processing.cpp:2816 (in thread 'msghand') 2020-06-22T15:46:56Z [msghand] (2) 'cs_main' in net_processing.cpp:2816 (in thread 'msghand') Assertion failed: detected inconsistent lock order for 'cs_main' in net_processing.cpp:2816 (in thread 'msghand'), details in debug log. Process 131393 stopped * thread #15, name = 'b-msghand', stop reason = signal SIGABRT frame #0: 0x00007ffff775c18b libc.so.6`__GI_raise(sig=2) at raise.c:51:1 (lldb) bt * thread #15, name = 'b-msghand', stop reason = signal SIGABRT * frame #0: 0x00007ffff775c18b libc.so.6`__GI_raise(sig=2) at raise.c:51:1 frame #1: 0x00007ffff773b859 libc.so.6`__GI_abort at abort.c:79:7 frame #2: 0x0000555555e5b196 bitcoind`(anonymous namespace)::potential_deadlock_detected(mismatch=0x00007fff99ff6f30, s1=size=2, s2=size=2, lock_location=0x00007fff99ff7010) at sync.cpp:134:9 frame #3: 0x0000555555e5a1b1 bitcoind`(anonymous namespace)::push_lock(c=0x0000555556379220, locklocation=0x00007fff99ff7010) at sync.cpp:158:13 frame #4: 0x0000555555e59e8a bitcoind`EnterCritical(pszName="cs_main", pszFile="net_processing.cpp", nLine=2816, cs=0x0000555556379220, fTry=false) at sync.cpp:177:5 frame #5: 0x00005555555b0500 bitcoind`UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::Enter(this=0x00007fff99ff8c20, pszName="cs_main", pszFile="net_processing.cpp", nLine=2816) at sync.h:134:9 frame #6: 0x00005555555b017f bitcoind`UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::UniqueLock(this=0x00007fff99ff8c20, mutexIn=0x0000555556379220, pszName="cs_main", pszFile="net_processing.cpp", nLine=2816, fTry=false) at sync.h:160:13 frame #7: 0x00005555556aa57e bitcoind`ProcessMessage(pfrom=0x00007fff90001180, msg_type=error: summary string parsing error, vRecv=0x00007fff9c005ac0, nTimeReceived=1592840815980751, chainparams=0x00005555564b7110, chainman=0x0000555556380880, mempool=0x0000555556380ae0, connman=0x000055555657aa20, banman=0x00005555565167b0, interruptMsgProc=0x00005555565cae90) at net_processing.cpp:2816:9 ``` ACKs for top commit: laanwj: ACK 0ecff9dd3418e8c18fa423ba53e9cab1df8be553 vasild: ACK 0ecff9dd Tree-SHA512: ff285de8dd3198b5b33c4bfbdadf9b1448189c96143b9696bc4f41c07e784c00851ec169cf3ed45cc325f3617ba6783620803234f57fcce28bf6bc3d6a7234fb
2020-07-15[tests] Remove unnecessary cs_mains in denialofservice_testsMatt Corallo
9fdf05d70cac4a62d1aeeb4299e2c3a9a866f8af resolved some lock inversion warnings in denialofservice_tests, but left in a number of cs_main locks that are unnecessary (introducing lock inversion warnings in future changes).
2020-07-15Merge #19512: p2p: banscore updates to gui, tests, release notesWladimir J. van der Laan
fa108d6a757838225179a8df942cfb6d99c98c90 test: update tests for peer discouragement (Jon Atack) 1a9f462caa63fa16d7b4415312d2032a42b3fe0b gui, doc: rm Ban Score in GUI Peers window/release notes updates (Jon Atack) Pull request description: This is the third `-banscore` PR in the mini-series described in #19464. See that PR for the intention and reasoning. - no longer display "Ban Score" in the GUI peers window and add a release note, plus release note fixups per https://github.com/bitcoin/bitcoin/pull/19464#pullrequestreview-447452052 - update tests (`src/test/denialofservice_tests.cpp` and `test/functional/p2p_leak.py`) from banning to discouragement and per https://github.com/bitcoin/bitcoin/pull/19464#issuecomment-658052518 ACKs for top commit: jnewbery: ACK fa108d6a757838225179a8df942cfb6d99c98c90 laanwj: ACK fa108d6a757838225179a8df942cfb6d99c98c90 Tree-SHA512: 58a449b3f47b8cb5490b34e4442ee8675bfad1ce48af4e4fd5c67715b0c1a596fb8e731d42e576b4c3b64627f76e0a68cbb1da9ea9f588a5932fe119baf40d50
2020-07-15Merge #19296: tests: Add fuzzing harness for AES{CBC,}256{Encrypt,Decrypt}, ↵Wladimir J. van der Laan
poly1305_auth, CHKDF_HMAC_SHA256_L32, ChaCha20 and ChaCha20Poly1305AEAD cca7c577d5d80293cb12de1048f3edd680ac4fad tests: Add fuzzing harness for ChaCha20Poly1305AEAD (practicalswift) 2fc4e5916c1c35902a32830c3f199a308a66bea0 tests: Add fuzzing harness for ChaCha20 (practicalswift) e9e8aac029acffb5e4cc5c2556f23cdfdcf9bb09 tests: Add fuzzing harness for CHKDF_HMAC_SHA256_L32 (practicalswift) ec86ca1aaae388cefa2da9904785cee2d550b3d1 tests: Add fuzzing harness for poly1305_auth(...) (practicalswift) 4cee53bba722a480ccd6472d2ffe9b0001394dd9 tests: Add fuzzing harness for AES256CBCEncrypt/AES256CBCDecrypt (practicalswift) 9352c3232594f953d2db11c1e140be3f7f9fbae4 tests: Add fuzzing harness for AES256Encrypt/AES256Decrypt (practicalswift) Pull request description: Add fuzzing harness for `AES{CBC,}256{Encrypt,Decrypt}`, `poly1305_auth`, `CHKDF_HMAC_SHA256_L32`, `ChaCha20` and `ChaCha20Poly1305AEAD`. See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) ACKs for top commit: laanwj: ACK cca7c577d5d80293cb12de1048f3edd680ac4fad Tree-SHA512: cff9acefe370c12a3663aa55145371df835479c6ab8f6d81bbf84e0f81a9d6b0d94e45ec545f9dd5e1702744eaa7947a1f4ffed0171f446fc080369161afd740
2020-07-15tests: Add fuzzing harness for CBlockPolicyEstimator::{Read,Write} ↵practicalswift
(policy/fees.h)
2020-07-15tests: Add fuzzing harness for ↵practicalswift
CBufferedFile::{SetPos,GetPos,GetType,GetVersion} (stream.h)
2020-07-15tests: Add fuzzing harness for LoadExternalBlockFile(...) (validation.h)practicalswift
2020-07-15tests: Add fuzzing harness for CBufferedFile (streams.h)practicalswift
2020-07-15tests: Add fuzzing harness for CAutoFile (streams.h)practicalswift
2020-07-15tests: Add serialization/deserialization fuzzing helpers ↵practicalswift
WriteToStream(…)/ReadFromStream(…)
2020-07-15tests: Add FuzzedAutoFileProvider which provides a CAutoFile interface to ↵practicalswift
FuzzedDataProvider
2020-07-15tests: Add FuzzedFileProvider which provides a FILE* interface to ↵practicalswift
FuzzedDataProvider using fopencookie
2020-07-15Merge #19491: util: Make Assert work with any valueMarcoFalke
fa5363538125d996ae5cede55f7f05e88701ace2 util: Make Assert work with any value (MarcoFalke) Pull request description: Goal is to avoid compile failures ACKs for top commit: jonatack: ACK fa5363538125d996ae5cede55f7f05e88701ace2 ryanofsky: Code review ACK fa5363538125d996ae5cede55f7f05e88701ace2. Looks like if argument is an lvalue this effectively does: Tree-SHA512: a5cf47a8bb2fa1bd8b8895774f33de50ad803165d6f7b520351be1cfcd5612d5d97c51d118461331d30640186c470879e5ad19e3333e09e72685c5e4e4f23079
2020-07-14refactor: replace CConnman pointers by references in net_processing.cppSebastian Falbesoner
2020-07-14test: update tests for peer discouragementJon Atack
2020-07-14Merge #19464: net: remove -banscore configuration optionMarcoFalke
06059b0c2a6c2db70c87a7715f8a344a13400fa1 net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD (Jon Atack) 1d4024bca8086cceff7539dd8c15e0b7fe1cc5ea net: remove -banscore configuration option (Jon Atack) Pull request description: per https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652684340, https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592. Edit: now split into 3 straightforward PRs: - net: remove -banscore configuration option (this PR) - rpc: deprecate banscore field in getpeerinfo (#19469, *merged*) - gui: no longer display banscores (TBA in the gui repo) ACKs for top commit: MarcoFalke: review ACK 06059b0c2a6c2db70c87a7715f8a344a13400fa1 đź“™ vasild: ACK 06059b0c Tree-SHA512: 03fad249986e0896697033fbb8ba2cbfaae7d7603b1fb2a38b3d41db697630d238623f4d732b9098c82af249ce5a1767dd432b7ca0fec10544e23d24fbd57c50
2020-07-13test: Remove duplicate NodeContext hacksRussell Yanofsky
Qt tests currently are currently using two NodeContext structs at the same time, one in interfaces::NodeImpl::m_context, and the other in BasicTestingSetup::m_node, and the tests have hacks transferring state between them. Fix this by getting rid of the NodeImpl::m_context struct and making it a pointer. This way a common BitcoinApplication object can be used for all qt tests, but they can still have their own testing setups. Non-test code is changing but non-test behavior is still the same as before. Motivation for this PR is to be able to remove the "std::move(test.m_node.connman)" and mempool hacks for swapping individual NodeContext members in Qt tests, because followup PR #19099 adds yet another member (wallet_client) that needs to be swapped. After this change, the whole NodeContext struct can be swapped instead of individual members, so the workarounds are less fragile and invasive.
2020-07-11net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLDJon Atack
and move it from validation to net processing.
2020-07-11net: remove -banscore configuration optionJon Atack