aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
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-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
2020-07-11util: Make Assert work with any valueMarcoFalke
2020-07-11wallet: Fix typo in comments; Simplify assertMarcoFalke
2020-07-11Merge #19222: tests: Add fuzzing harness for BanManMarcoFalke
97846d7f5b47ef77469b9f961db77f770e8bcc0f tests: Add fuzzing harness for BanMan (practicalswift) deba199f1c88c2e5f754b0a4ec43b9ef28de8352 tests: Add ConsumeSubNet(...). Move and increase coverage in ConsumeNetAddr(...). (practicalswift) Pull request description: Add fuzzing harness for `BanMan`. 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 :) Top commit has no ACKs. Tree-SHA512: f4126c15bbb77638833367d73f58193c8f05d16bed0b1d6c33b39387d5b610ff34af78cd721adb51778062ce3ac5e79756d1c3895ef54c6c80c61dcf056e94ff
2020-07-11Merge #19474: doc: Use precise permission flags where possibleMarcoFalke
fab558612278909df93bdf88f5727b04f13aef0f doc: Use precise permission flags where possible (MarcoFalke) Pull request description: Instead of mentioning the all-encompassing `-whitelist*` settings, change the docs to mention the exact permission flag that will influence the behaviour. This is needed because in the future, the too-broad `-whitelist*` settings (they either include *all* permission flags or apply to *all* peers) might be deprecated to require the permission flags to be enumerated. Alternatively, in the future there could be an RPC to set the net permission flags on an existing connection, in which case the `-whitelist*` terminology is of no help. ACKs for top commit: jnewbery: reACK fab558612278909df93bdf88f5727b04f13aef0f fjahr: Code review ACK fab558612278909df93bdf88f5727b04f13aef0f jonatack: ACK fab558612278909df93bdf88f5727b04f13aef0f Tree-SHA512: c7dea3e577d90103bb2b0ffab7b7c8640b388932a3a880f69e2b70747fc9213dc1f437085671fd54c902ec2a578458b8a2fae6dbe076642fb88efbf9fa9e679c
2020-07-10Merge #19140: tests: Avoid fuzzer-specific nullptr dereference in libevent ↵MarcoFalke
when handling PROXY requests 20d31bdd92cc2ad9b8d26ed80da73bbcd6016144 tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests (practicalswift) Pull request description: Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering a `nullptr` dereference. Split out from #19074 as suggested by MarcoFalke. The dereference (`req->evcon->http_server`) takes place in `evhttp_parse_request_line` and is a consequence of our hacky but necessary use of the internal function `evhttp_parse_firstline_` in the `http_request` fuzzing harness. The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path. `" http:// HTTP/1.1\n"` was a crashing input prior to this workaround. Before this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input AddressSanitizer:DEADLYSIGNAL ================================================================= ==27905==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000108 (pc 0x55a169b7e053 bp 0x7ffd452f1160 sp 0x7ffd452f10e0 T0) ==27905==The signal is caused by a READ memory access. ==27905==Hint: address points to the zero page. #0 0x55a169b7e053 in evhttp_parse_request_line depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:1883:37 #1 0x55a169b7d9ae in evhttp_parse_firstline_ depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:2041:7 #2 0x55a1687f624e in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/test/fuzz/http_request.cpp:51:9 … $ echo $? 1 ``` After this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input Executed input in 0 ms *** *** NOTE: fuzzing was not performed, you have only *** executed the target code on a fixed set of inputs. *** $ echo $? 0 ``` 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 :) Top commit has no ACKs. Tree-SHA512: 7a6b68e52cbcd6c117487e74e47760fe03566bec09b0bb606afb3b652edfd22186ab8244e8e27c38cef3fd0d4a6c237fe68b2fd22e0970c349e4ab370cf3e304
2020-07-10Merge #18638: net: Use mockable time for ping/pong, add testsMarcoFalke
fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 net: Use mockable time for ping/pong, add tests (MarcoFalke) faab4aaf2fa1153c6d76efc8113fa01b06943ece util: Add count_microseconds helper (MarcoFalke) Pull request description: Switch `CNode::m_ping_start` and `CNetMessage::m_time` to mockable time, so that tests can be added. Mockable time is also type-safe, since it uses `std::chrono` ACKs for top commit: jonatack: Code review re-ACK fa33654 re-read code, verified rebase per `git range-diff 4b5c919 fa94d6f fa33654`, previous tested ACKs still valid troygiorshev: ACK fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 Tree-SHA512: 7d632bd6019ce7c882029e71b667a61517e783af82755a85dd979ef09380934e172dec8b8f91d57b200a30a6e096aeaf01f19fee7f3aed0e0e871c72eb44d70e
2020-07-10doc: Use precise permission flags where possibleMarcoFalke
2020-07-09Merge #19191: net: Extract download permission from nobanMarcoFalke
fa0540cd46eaf44d9e1a9f91c3a937986826c4fa net: Extract download permission from noban (MarcoFalke) Pull request description: It should be possible to grant nodes in a local network (e.g. home, university, enterprise, ...) permission to download blocks even after the maxuploadtarget is hit. Currently this is only possible by setting the `noban` permission, which has some adverse effects, especially if the peers can't be fully trusted. Fix this by extracting a `download` permission from `noban`. ACKs for top commit: jonatack: ACK fa0540c Sjors: re-utACK fa0540cd46eaf44d9e1a9f91c3a937986826c4fa Tree-SHA512: 255566baa43ae925d93f5d0a3aa66b475a556d1590f662a88278a4872f16a1a05739a6119ae48a293011868042e05cb264cffe5822a50fb80db7333bf44376d9
2020-07-09Merge #19314: refactor: Use uint16_t instead of unsigned shortWladimir J. van der Laan
1cabbddbca615b26aa4510c75f459c28d6fe0afd refactor: Use uint16_t instead of unsigned short (Aaron Hook) Pull request description: I wanted to see if the `up for grabs` label works and looked at PR #17822 originally opend by ahook I saw it had many acks for example by jonatack and practicalswift but needed rebasing. So I checked out the remote branch rebased it resolved three conflicts and continued the rebase. Hope everything is as expected (: ACKs for top commit: sipsorcery: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd. practicalswift: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd -- patch looks correct :) laanwj: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd hebasto: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 0e6bf64f274aae5dacb188358b4d5f65ccb207d4f70922f039bc4ed7934709418ddad19f8bfb7462517427837c3d2bb3f86ef284bb40e87119aad2a1e148d9d6
2020-07-09net: Extract download permission from nobanMarcoFalke
2020-07-08tests: Add fuzzing harness for BanManpracticalswift
2020-07-08tests: Add ConsumeSubNet(...). Move and increase coverage in ↵practicalswift
ConsumeNetAddr(...).
2020-07-07Merge #19219: Replace automatic bans with discouragement filterPieter Wuille
2ad58381fffb33d611abf900b73d9e6b5a4e35f8 Clean up separated ban/discourage interface (Pieter Wuille) b691f2df5f7d443c0c9ee056ab94aa0fc19566d5 Replace automatic bans with discouragement filter (Pieter Wuille) Pull request description: This patch improves performance and resource usage around IP addresses that are banned for misbehavior. They're already not actually banned since #14929, as connections from them are still allowed, but they are preferred for eviction if the inbound connection slots are full. Stop treating these like manually banned IP ranges, and instead just keep them in a rolling Bloom filter of misbehaving nodes, which isn't persisted to disk or exposed through the ban framework. The effect remains the same: preferred for eviction, avoided for outgoing connections, and not relayed to other peers. Also change the name of this mechanism to "discouraged" to better reflect reality. ACKs for top commit: naumenkogs: utACK 2ad58381fffb33d611abf900b73d9e6b5a4e35f8 amitiuttarwar: code review ACK 2ad58381ff jonatack: ACK 2ad5838 per changes since last review `git range-diff 3276c14 1f7e0ca 2ad5838` jnewbery: Code review ACK 2ad58381fffb33d611abf900b73d9e6b5a4e35f8 Tree-SHA512: 5dedef401d9cbfa026812651303e6286223563dbeed7a10766ed536ac9e3f29ed4bd0df29cc6deadceeb35cbe9f066346add14ef0833958ca9f93d123fe7aab5
2020-07-06Merge #19328: Add gettxoutsetinfo hash_type optionMarcoFalke
40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a test: Test gettxouttsetinfo hash_type option (Fabian Jahr) f17a4d1c4ddce6935a353004898fb4e8618a213e rpc: Add hash_type NONE to gettxoutsetinfo (Fabian Jahr) a712cf6f6801157667fcf36d1c498b6fff6d328a rpc: gettxoutsetinfo can specify hash_type (only legacy option for now) (Fabian Jahr) 605884ef21318fc3f326dbdf4901cb353ba63fab refactor: Extract GetBogoSize function (Fabian Jahr) Pull request description: This is another intermediate part of the Coinstats Index (tracked in #18000). Sjors suggested [here](https://github.com/bitcoin/bitcoin/pull/18000#issuecomment-641423019) that the part of the changes in #19145 that don't rely on the new `hash_type` muhash, i.e. that are for `hash_type=none`, could be merged separately from everything involving muhash. So these changes are extracted from #19145 here and can be merged without any other requirements. Building the index with no UTXO set hash is still valuable because `gettxoutsetinfo` can still be used to audit the `total_amount` for example. By itself this PR is not a huge improvement, `hash_type=none` is speeding up `gettxoutsetinfo` by about 10%, but it enables the implementation of an index on top of it in a follow-up and that means large parts of the index code of Coinstats Index can be merged while reviews for the hashing algorithm might take longer. ACKs for top commit: MarcoFalke: ACK 40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a 🖨 Sjors: tACK 40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a Tree-SHA512: 3964c2b8eed427511b1aa9b2ef285dff27dc4d1537d72c3911e435b6e6b40912232da4acb3a09bd19a0372ddffa44103388d8a650169d95a4a727b970d210add
2020-07-04Merge #19277: util: Add Assert identity functionMarcoFalke
fab80fef61ddd4afeff6e497c7e76bffcd05e8a4 refactor: Remove unused EnsureChainman (MarcoFalke) fa34587f1c811d99200453b0936219c473f514b0 scripted-diff: Replace EnsureChainman with Assert in unit tests (MarcoFalke) fa6ef701adba1cb48535cac25fd43c742a82e40d util: Add Assert identity function (MarcoFalke) fa457fbd3387661e1973a8f4e5cc2def79e0c625 move-only: Move NDEBUG compile time check to util/check (MarcoFalke) Pull request description: The utility function is primarily useful to dereference pointer types, which are known to be not null at that time. For example, the ArgsManager is known to exist when the wallets are started: https://github.com/bitcoin/bitcoin/pull/18923/files#diff-fdb2a1a1d8bc790fcddeb6cf5a42ac55R503 . Instead of silently relying on that assumption, `Assert` can be used to abort the program and avoid UB should the assumption ever be violated. ACKs for top commit: promag: Tested ACK fab80fef61ddd4afeff6e497c7e76bffcd05e8a4. ryanofsky: Code review ACK fab80fef61ddd4afeff6e497c7e76bffcd05e8a4 Tree-SHA512: 830fba10152ba17d47c4dd42809c7e26f9fe6d38e17a2d5b3f054fd644a5c4c9841286ac421ec9bb28cea9f5faeb659740fcf00de6cc589d423fee7694c42d16
2020-07-03Clean up separated ban/discourage interfacePieter Wuille
2020-07-03Replace automatic bans with discouragement filterPieter Wuille
This patch improves performance and resource usage around IP addresses that are banned for misbehavior. They're already not actually banned, as connections from them are still allowed, but they are preferred for eviction if the inbound connection slots are full. Stop treating these like manually banned IP ranges, and instead just keep them in a rolling Bloom filter of misbehaving nodes, which isn't persisted to disk or exposed through the ban framework. The effect remains the same: preferred for eviction, avoided for outgoing connections, and not relayed to other peers. Also change the name of this mechanism to better reflect reality; they're not banned, just discouraged. Contains release notes and several interface improvements by John Newbery.
2020-07-03Merge #19215: psbt: Include and allow both non_witness_utxo and witness_utxo ↵Samuel Dobson
for segwit inputs 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 tests: Check that segwit inputs in psbt have both UTXO types (Andrew Chow) 46004790588c24174a0bec49b540d158ce163ffd psbt: always put a non_witness_utxo and don't remove it (Andrew Chow) 5279d8bc07d601fe6a67ad665fbc7591fe73c7de psbt: Allow both non_witness_utxo and witness_utxo (Andrew Chow) 72f6bec1da198764d4648a10a61c485e7ab65e9e rpc: show both UTXOs in decodepsbt (Andrew Chow) Pull request description: Due to recent changes to hardware wallets, the full previous transaction will need to be provided for segwit inputs. Since some software may be checking for the existence of a `witness_utxo` to determine whether to produce a segwit signature, we keep that field to ease the transition. Because all of the sanity checks implemented by the `IsSane` functions were related to having mixed segwit and non-segwit data in a PSBT, those functions are removed as those checks are no longer proper. Some tests are updated/removed to accommodate this and a simple test added to check that both UTXOs are being added to segwit inputs. As discussed in the wallet IRC meeting, our own signer will not require `non_witness_utxo` for segwit inputs. ACKs for top commit: Sjors: utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 (didn't retest compared to 836d6fc, but fortunately HWI's CI tracks our master branch, with a bunch of hardware wallet simulators) ryanofsky: Code review re-ACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3. No changes since last review, but now I understand the context better. I think it would good to improve the comments as suggested https://github.com/bitcoin/bitcoin/pull/19215#discussion_r447889473 and maybe refer to meshcollider: utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 Tree-SHA512: ccc1fd3c16ac3859f5aca4fa489bd40f68be0b81bbdc4dd51188bbf28827a8642dc8b605a37318e5f16cf40f1c4910052dace2f27eca21bb58435f02a443e940
2020-07-01Merge #19028: test: Set -logthreadnames in unit testsWladimir J. van der Laan
99993489da9bc003b823bcab10e5f5297b369431 test: Set -logthreadnames in unit tests (MarcoFalke) fa4ea997b4da1ae0afafba223fff9efbeefaf555 init: Setup scheduler in tests and init in exactly the same way (MarcoFalke) Pull request description: Generally the unit tests are single threaded, with the exception of the script check threads, the schedule, and optionally indexer threads. Like the functional tests, the thread name can serve additional debug information, so set `-logthreadnames` in unit tests. Can be tested with ``` ./src/test/test_bitcoin -l test_suite -t validation_tests/test_combiner_all -- DEBUG_LOG_OUT ACKs for top commit: laanwj: ACK 99993489da9bc003b823bcab10e5f5297b369431 Tree-SHA512: 3bdbfc211da146da64b50b0826246aff5c611a84b69ab896a55b3c9d1adc92c5975da36ab92aee577df82e229c4326b477f4105bfdd1a5df4c9a0b018cf61602
2020-06-29Merge #19204: p2p: Reduce inv traffic during IBDMarcoFalke
fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 net: Avoid wasting inv traffic during IBD (MarcoFalke) fa06d7e93489e61078cfb95ab767c001536a6e10 refactor: block import implies IsInitialBlockDownload (MarcoFalke) faba65e696a88e5626e587f4e63fa15500cbe4d0 Add ChainstateManager::ActiveChainstate (MarcoFalke) fabf3d64ff2bd14f762810316144bb9fd69c517c test: Add FeeFilterRounder test (MarcoFalke) Pull request description: Tx-inv messages are ignored during IBD, so it would be nice if we told peers to not send them in the first place. Do that by sending two `feefilter` messages: One when the connection is made (and the node is in IBD), and another one when the node leaves IBD. ACKs for top commit: jamesob: ACK fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 ([`jamesob/ackr/19204.1.MarcoFalke.p2p_reduce_inv_traffic_d`](https://github.com/jamesob/bitcoin/tree/ackr/19204.1.MarcoFalke.p2p_reduce_inv_traffic_d)) naumenkogs: utACK fa525e4 gzhao408: ACK https://github.com/bitcoin/bitcoin/commit/fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 jonatack: re-ACK fa525e4 checked diff `git range-diff 19612ca fa8a66c fa525e4`, re-reviewed, ran tests, ran a custom p2p IBD behavior test at https://github.com/jonatack/bitcoin/commit/9321e0f223ea87d21f6fa42b61bcc8d40a0943de. hebasto: re-ACK fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72, only rebased since the [previous](https://github.com/bitcoin/bitcoin/pull/19204#pullrequestreview-429519667) review (verified with `git range-diff`). Tree-SHA512: 2c22a5def9822396fca45d808b165b636f1143c4bdb2eaa5c7e977f1f18e8b10c86d4c180da488def38416cf3076a26de15014dfd4d86b2a7e5af88c74afb8eb
2020-06-28Merge #19114: scripted-diff: TxoutType C++11 scoped enum classMarcoFalke
fa32adf9dc25540ad27f5b82654c7057d7738627 scripted-diff: TxoutType C++11 scoped enum class (MarcoFalke) fa95a694c492b267e4038674fd3f338dd215ab48 doc: Update outdated txnouttype documentation (MarcoFalke) fa58469c770d8c935a86462634e4e8cd806aa6e3 rpc: Properly use underlying type in GetAllOutputTypes (MarcoFalke) fa41c657022b8f99c8e6718a0e33c5838c412a0b rpc: Simplify GetAllOutputTypes with the Join helper (MarcoFalke) Pull request description: Non-scoped enums can accidentally and silently decay into an integral type. Also, the symbol names of the keys are exported to the surrounding (usually global) namespace. Fix both issues by switching to an `enum class TxoutType` in a (mostly) scripted-diff. ACKs for top commit: practicalswift: ACK fa32adf9dc25540ad27f5b82654c7057d7738627 -- patch looks correct hebasto: re-ACK fa32adf9dc25540ad27f5b82654c7057d7738627, since fa5997bd6fc82e16b597ea96e3c5c665f1f174ab (https://github.com/bitcoin/bitcoin/pull/19114#pullrequestreview-421425198) rebased only (verified with `git range-diff`). Tree-SHA512: f42a9db47f9be89fa4bdd8d2fb05a16726286d8b12e3d87327b67d723f91c7d5a57deb4b2ddae9e1d16fee7a5f8c00828b6dc8909c5db680fc5e0a3cf07cd465
2020-06-26Merge #19366: tests: Provide main(...) function in fuzzer. Allow building ↵MarcoFalke
uninstrumented harnesses with --enable-fuzz. 1087807b2bc56b9c7e7a5471c83f6ecfae79b048 tests: Provide main(...) function in fuzzer (practicalswift) Pull request description: Provide `main(...)` function in fuzzer. Allow building uninstrumented harnesses with only `--enable-fuzz`. This PR restores the behaviour to how things worked prior to #18008. #18008 worked around an macOS specific issue but did it in a way which unnecessarily affected platforms not in need of the workaround :) Before this patch: ``` # Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation) $ ./configure --enable-fuzz $ make CXXLD test/fuzz/span /usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/Scrt1.o: In function `_start': (.text+0x20): undefined reference to `main' collect2: error: ld returned 1 exit status Makefile:7244: recipe for target 'test/fuzz/span' failed make[2]: *** [test/fuzz/span] Error 1 make[2]: *** Waiting for unfinished jobs.... $ ``` After this patch: ``` # Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation) $ ./configure --enable-fuzz $ make $ echo foo | src/test/fuzz/span $ ``` The examples above show the change in non-macOS functionality. macOS functionality is unaffected by this patch. ACKs for top commit: MarcoFalke: ACK 1087807b2bc56b9c7e7a5471c83f6ecfae79b048 Tree-SHA512: 9c16ea32ffd378057c4fae9d9124636d11e3769374d340f68a1b761b9e3e3b8a33579e60425293c96b8911405d8b96ac3ed378e669ea4c47836af06892aca73d
2020-06-25tests: Provide main(...) function in fuzzerpracticalswift
2020-06-25tests: Add fuzzing harness for ChaCha20Poly1305AEADpracticalswift
2020-06-25tests: Add fuzzing harness for ChaCha20practicalswift
2020-06-25tests: Add fuzzing harness for CHKDF_HMAC_SHA256_L32practicalswift
2020-06-25tests: Add fuzzing harness for poly1305_auth(...)practicalswift
2020-06-25tests: Add fuzzing harness for AES256CBCEncrypt/AES256CBCDecryptpracticalswift
2020-06-25tests: Add fuzzing harness for AES256Encrypt/AES256Decryptpracticalswift