aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2021-02-12simplify ChainstateManager::SnapshotBlockhash() return semanticsJames O'Beirne
Don't return null snapshotblockhash values to avoid caller complexity/confusion.
2021-02-12chainparams: add allowed assumeutxo valuesJames O'Beirne
Values for mainnet and testnet will be specified in a follow-up PR that can be scrutinized accordingly. This structure is required for use in snapshot activation logic.
2021-02-12Merge #21064: refactor: use std::shared_mutex & remove Boost ThreadWladimir J. van der Laan
060a2a64d40d75fecb60b7d2b9946a67e46aa6fc ci: remove boost thread installation (fanquake) 06e1d7d81d5a56d136c6fc88f09a2b0654a164f9 build: don't build or use Boost Thread (fanquake) 7097add83c8596f81be9edd66971ffd2486357eb refactor: replace Boost shared_mutex with std shared_mutex in sigcache (fanquake) 8e55981ef834490c438436719f95cbaf888c4914 refactor: replace Boost shared_mutex with std shared_mutex in cuckoocache tests (fanquake) Pull request description: This replaces `boost::shared_mutex` and `boost::unique_lock` with [`std::shared_mutex`](https://en.cppreference.com/w/cpp/thread/shared_mutex) & [`std::unique_lock`](https://en.cppreference.com/w/cpp/thread/unique_lock). Even though [some concerns were raised](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-726214696) in #16684 with regard to `std::shared_mutex` being unsafe to use across some glibc versions, I still think this change is an improvement. As I mentioned in #21022, I also think trying to restrict standard library feature usage based on bugs in glibc is not only hard to do, but it's not currently clear exactly how we do that in practice (does it also extend to patching out use in our dependencies, should we be implementing more runtime checks for features we are using, when do we consider an affected glibc "old enough" not to worry about? etc). If you take a look through the [glibc bug tracker](https://sourceware.org/bugzilla/describecomponents.cgi?product=glibc) you'll no doubt find plenty of (active) bug reports for standard library code we already using. Obviously not to say we shouldn't try and avoid buggy code where possible. Two other points: [Cory mentioned in #21022](https://github.com/bitcoin/bitcoin/pull/21022#issuecomment-769274179): > It also seems reasonable to me to worry that boost hits the same underlying glibc bug, and we've just not happened to trigger the right conditions yet. Moving away from Boost to the standard library also removes the potential for differences related to Boosts configuration. Boost has multiple versions of `shared_mutex`, and what you end up using, and what it's backed by depends on: * The version of Boost. * The platform you're building for. * Which version of `BOOST_THREAD_VERSION` is defined: (2,3,4 or 5) default=2. (see [here](https://www.boost.org/doc/libs/1_70_0/doc/html/thread/build.html#thread.build.configuration) for some of the differences). * Is `BOOST_THREAD_V2_SHARED_MUTEX` defined? (not by default). If so, you might get the ["less performant, but more robust"](https://github.com/boostorg/thread/issues/230#issuecomment-475937761) version of `shared_mutex`. A lot of these factors are eliminated by our use of depends, but users will have varying configurations. It's also not inconceivable to think that a distro, or some package manager might start defining something like `BOOST_THREAD_VERSION=3`. Boost tried to change the default from 2 to 3 at one point. With this change, we no longer use Boost Thread, so this PR also removes it from depends, the build system, CI etc. Previous similar PRs were #19183 & #20922. The authors are included in the commits here. Also related to #21022 - pthread sanity checking. ACKs for top commit: laanwj: Code review ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc vasild: ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc Tree-SHA512: 572d14d8c9de20bc434511f20d3f431836393ff915b2fe9de5a47a02dca76805ad5c3fc4cceecb4cd43f3ba939a0508178c4e60e62abdbaaa6b3e8db20b75b03
2021-02-11Merge #20370: fuzz: version handshakeMarcoFalke
fabce459bb44e90dc7ae9c44eeedab707435af5b fuzz: version handshake (MarcoFalke) Pull request description: Not fuzzing the version handshake will limit fuzz coverage ACKs for top commit: practicalswift: cr ACK fabce459bb44e90dc7ae9c44eeedab707435af5b: patch looks very much correct Tree-SHA512: 4091d27d39edee781d033e471b352084bb54df250d0890e4821a325926a44dff9b26a2614d67dd0529f73bd366b075d7a0a1a570c2837de286a1b93a59a8fb91
2021-02-11Merge #21062: refactor: return MempoolAcceptResult from ATMPMarcoFalke
53e716ea119658c28935fee24eb50090907c500e [refactor] improve style for touched code (gzhao408) 174cb5330af4b09f3a66974d3bae783ea43b190e [refactor] const ATMPArgs and non-const Workspace (gzhao408) f82baf0762f60c2ca5ffc339b095f9271d7c2f33 [refactor] return MempoolAcceptResult (gzhao408) 9db10a55061e09021ff8ea1d6637d99f7959035f [refactor] clean up logic in testmempoolaccept (gzhao408) Pull request description: This is the first 4 commits of #20833, and does refactoring only. It should be relatively simple to review, and offers a few nice things: - It makes accessing values that don't make sense (e.g. fee) when the tx is invalid an error. - Returning `MempoolAcceptResult` from ATMP makes the interface cleaner. The caller can get a const instead of passing in a mutable "out" param. - We don't have to be iterating through a bunch of lists for package validation, we can just return a `std::vector<MempoolAcceptResult>`. - We don't have to refactor all ATMP call sites again if/when we want to return more stuff from it. ACKs for top commit: MarcoFalke: ACK 53e716ea119658c28935fee24eb50090907c500e 💿 jnewbery: Code review ACK 53e716ea119658c28935fee24eb50090907c500e ariard: Code Review ACK 53e716e, I did tweak a bit the touched paths to see if we had good test coverage. Didn't find holes. Tree-SHA512: fa6ec324a08ad9e6e55948615cda324cba176255708bf0a0a0f37cedb7a75311aa334ac6f223be7d8df3c7379502b1081102b9589f9a9afa1713ad3d9ab3c24f
2021-02-11Merge #20788: net: add RAII socket and use it instead of bare SOCKETWladimir J. van der Laan
615ba0eb96cf131364c1ceca9d3dedf006fa1e1c test: add Sock unit tests (Vasil Dimov) 7bd21ce1efc363b3e8ea1d51dd1410ccd66820cb style: rename hSocket to sock (Vasil Dimov) 04ae8469049e1f14585aabfb618ae522150240a7 net: use Sock in InterruptibleRecv() and Socks5() (Vasil Dimov) ba9d73268f9585d4b9254adcf54708f88222798b net: add RAII socket and use it instead of bare SOCKET (Vasil Dimov) dec9b5e850c6aad989e814aea5b630b36f55d580 net: move CloseSocket() from netbase to util/sock (Vasil Dimov) aa17a44551c03b00a47854438afe9f2f89b6ea74 net: move MillisToTimeval() from netbase to util/time (Vasil Dimov) Pull request description: Introduce a class to manage the lifetime of a socket - when the object that contains the socket goes out of scope, the underlying socket will be closed. In addition, the new `Sock` class has a `Send()`, `Recv()` and `Wait()` methods that can be overridden by unit tests to mock the socket operations. The `Wait()` method also hides the `#ifdef USE_POLL poll() #else select() #endif` technique from higher level code. ACKs for top commit: laanwj: Re-ACK 615ba0eb96cf131364c1ceca9d3dedf006fa1e1c jonatack: re-ACK 615ba0eb96cf131364c1ceca9d3dedf006fa1e1c Tree-SHA512: 3003e6bc0259295ca0265ccdeb1522ee25b4abe66d32e6ceaa51b55e0a999df7ddee765f86ce558a788c1953ee2009bfa149b09d494593f7d799c0d7d930bee8
2021-02-11fuzz: version handshakeMarcoFalke
2021-02-11Merge #20211: Use -Wswitch for TxoutType where possibleMarcoFalke
fa650ca7f19307a9237e64ac311488c8947fc12a Use -Wswitch for TxoutType where possible (MarcoFalke) fa59e0b5bd2aed8380cc9b9e52791f662aecd6a6 test: Add missing script_standard_Solver_success cases (MarcoFalke) Pull request description: This removes unused `default:` cases for all `switch` statements on `TxoutType` and adds the cases (`MULTISIG`, `NULL_DATA`, `NONSTANDARD`) to `ExtractDestination` for clarity. Also, the compiler is now able to use `-Wswitch`. ACKs for top commit: practicalswift: cr ACK fa650ca7f19307a9237e64ac311488c8947fc12a: patch looks correct and `assert(false);` is better than UB :) hebasto: ACK fa650ca7f19307a9237e64ac311488c8947fc12a, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 282458b6523bd8923a0c0f5c423d1db2dce2a2d1b1d1dae455415c6fc995bb41ce82c1f9b0a1c0dcc6d874d171e04c30eca585f147582f52c7048c140358630a
2021-02-11test: add Sock unit testsVasil Dimov
2021-02-11fuzz: Fail if message type is not fuzzedMarcoFalke
2021-02-11fuzz: Count message type fuzzers before main()MarcoFalke
2021-02-11Merge #20663: fuzz: Hide script_assets_test_minimizerMarcoFalke
fac726b1b8331b267973138bbd2bff5304774315 doc: Fixup docs in fuzz/script_assets_test_minimizer.cpp (MarcoFalke) fafca47adc2476f19f7926de4d55b64b0286e41c fuzz: Hide script_assets_test_minimizer (MarcoFalke) Pull request description: This is not an actual fuzz target. It is a hack to exploit the built-in capability of fuzz engines to measure coverage. ACKs for top commit: practicalswift: cr ACK fac726b1b8331b267973138bbd2bff5304774315: patch looks correct and touches only `src/test/fuzz/` Tree-SHA512: 0652dd8d9e95746b0906be4044467435d8204a34a30366ae9bdb75b9cb0788d429db7cedf2760fd543565d9d4f7ee206873ed10a29dd715a792a26337f65b53c
2021-02-09Merge #21052: refactor: Replace fs::unique_path with GetUniquePath(path) callsWladimir J. van der Laan
1bca2aa694cd85984c09699ae28daec313077462 Introduce GetUniquePath(base) helper method to replace boost::filesystem::unique_path() which is not available in std::filesystem. (Kiminuo) Pull request description: This PR makes it easier in #20744 to remove our dependency on the `boost::filesystem::unique_path()` function which does not have a direct equivalent in C++17. This PR attempts to re-implement `boost::filesystem::unique_path()` as `GetUniquePath(path)` but the implementations are not meant to be the same. Note: * Boost 1.75.0 implementation of `unique_path`: https://github.com/boostorg/filesystem/blob/9cab675b71e98706886a87afe7c19eb9da568961/src/unique_path.cpp#L235 * In the previous implementation, I attempted to add: ```cpp fs::path GetUniquePath(const fs::path& base) { FastRandomContext rnd; fs::path tmpFile = base / HexStr(rnd.randbytes(8)); return tmpFile; } ``` to `fs.cpp` but this leads to a circular dependency: "fs -> random -> logging -> fs". That is why the modified implementation adds a new file. ACKs for top commit: laanwj: Code review ACK 1bca2aa694cd85984c09699ae28daec313077462 ryanofsky: Code review ACK 1bca2aa694cd85984c09699ae28daec313077462. It's a simple change and extra test coverage is nice Tree-SHA512: f324bdf0e254160c616b5033c3ece33d87db23eb0135acee99346ade7b5cf0d30f3ceefe359a25a8e9b53ba8e4419f459c2bdd369e10fc0152ce95031d1f221c
2021-02-09[refactor] return MempoolAcceptResultgzhao408
This creates a cleaner interface with ATMP, allows us to make results const, and makes accessing values that don't make sense (e.g. fee when tx is invalid) an error.
2021-02-08test: Fix Windows cross buildHennadii Stepanov
2021-02-08doc: Fixup docs in fuzz/script_assets_test_minimizer.cppMarcoFalke
2021-02-08fuzz: Hide script_assets_test_minimizerMarcoFalke
Can be reviewed with --ignore-all-space
2021-02-05Merge #20839: fuzz: Avoid extraneous copy of input data, using Span<>Wladimir J. van der Laan
faf7d7418cf01cb04cd457bcc630654da958a777 fuzz: Avoid extraneous copy of input data, using Span<> (MarcoFalke) Pull request description: Seeing speedup here in the fuzz framework part (non-fuzz-target part). Speedup is only visible for input data larger than 100kB. ACKs for top commit: practicalswift: cr ACK faf7d7418cf01cb04cd457bcc630654da958a777: patch looks correct :) laanwj: Code review ACK faf7d7418cf01cb04cd457bcc630654da958a777 Tree-SHA512: 41af7118846e0dfee237a6d5269a6c7cfbc775d7bd1cc2a85814cb60f6c2b37fe7fd35f1a788d4f08e6e0202c48b71054b67d2931160c445c79fc59e5347dadf
2021-02-04Introduce GetUniquePath(base) helper method to replace ↵Kiminuo
boost::filesystem::unique_path() which is not available in std::filesystem.
2021-02-04Merge #20715: util: Add ArgsManager::GetCommand() and use it in bitcoin-walletMarcoFalke
fa61b9d1a68820758f9540653920deaeae6abe79 util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke) 7777105a24a36b62df35d12ecf6c6370671568c8 refactor: Move all command dependend checks to ExecuteWalletToolFunc (MarcoFalke) fa06bce4ac17f93decd4ee38c956e7aa55983f0d test: Add tests (MarcoFalke) fac05ccdade8b34c969b9cd9b37b355bc0aabf9c wallet: [refactor] Pass ArgsManager to WalletAppInit (MarcoFalke) Pull request description: This not only moves the parsing responsibility out from the wallet tool, but it also makes it easier to implement bitcoin-util #19937 Fixes: #20902 ACKs for top commit: ajtowns: ACK fa61b9d1a68820758f9540653920deaeae6abe79 fjahr: Code review ACK fa61b9d1a68820758f9540653920deaeae6abe79 Tree-SHA512: 79622b806e8bf9dcd0dc24a8a6687345710df57720992e83a41cd8d6762a6dc112044ebc58fcf6e8fbf45de29a79b04873c5b8c2494a1eaaf902a2884703e47b
2021-02-03fuzz: Avoid extraneous copy of input data, using Span<>MarcoFalke
2021-02-02refactor: replace Boost shared_mutex with std shared_mutex in cuckoocache testsfanquake
Co-authored-by: MarcoFalke falke.marco@gmail.com Co-authored-by: sinetek pitwuu@gmail.com
2021-02-01Merge #20464: refactor: Treat CDataStream bytes as uint8_tWladimir J. van der Laan
fa292724598c273867bc6dbf311f1440fe2541ba Remove redundant MakeUCharSpan wrappers (MarcoFalke) faf4aa2f47c0de4f3a0c5f5fe5b3ec32f611eefd Remove CDataStream::Init in favor of C++11 member initialization (MarcoFalke) fada14b948cac147198e3b685b5dd8cb72dc2911 Treat CDataStream bytes as uint8_t (MarcoFalke) fa8bdb048e65cae2d26bea3f991717a856e2fb39 refactor: Drop CDataStream constructors in favor of one taking a Span of bytes (MarcoFalke) faa96f841fe45bc49ebb6e07ac82a129fa9c40bf Remove unused CDataStream methods (MarcoFalke) Pull request description: Using `uint8_t` for raw bytes has a style benefit: * The signedness is clear from reading the code, as it does not depend on the architecture Other clean-ups in this pull include: * Remove unused methods * Constructor is simplified with `Span` * Remove `Init()` member in favor of C++11 member initialization ACKs for top commit: laanwj: code review ACK fa292724598c273867bc6dbf311f1440fe2541ba theStack: ACK fa292724598c273867bc6dbf311f1440fe2541ba 🍾 Tree-SHA512: 931ee28bd99843d7e894b48e90e1187ffb0278677c267044b3c0c255069d9bbd9298ab2e539b1002a30b543d240450eaec718ef4ee95a7fd4be0a295e926343f
2021-02-01Merge #21016: refactor: remove boost::thread_group usageWladimir J. van der Laan
dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f refactor: remove boost::thread_group usage (fanquake) Pull request description: Post #18710, there isn't much left using `boost::thread_group`, so should just be able to replace it with the standard library. This also removes the last use of `boost::thread_interrupted`. After this change, last piece of Boost Thread we'd be using is `boost::shared_mutex`. See the commentary [here](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-726214696) as to why it may be non-trivial to swap that for `std::shared_mutex` in the near future. Closes #17307 ACKs for top commit: laanwj: Code review re-ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f MarcoFalke: review ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f 🔁 jonatack: Non-expert code review ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f, also checked range-diff since last review and that local debug build is clean with gcc 10.2.1-6 on Debian Tree-SHA512: 5510e2d760cce824234207dc86b1551ca8f21cbf3a2ce753c0254a0d03ffd83c94e449aec202fb7bd76e6fc64df783a6b70a736b0add9ece3734bb9c8ce8fc2f
2021-02-01Merge #20749: [Bundle 1/n] Prune g_chainman usage related to ::LookupBlockIndexWladimir J. van der Laan
67c9a83df19c6e2a2edb32336879204e7770b4a7 style-only: Remove redundant sentence in ActivateBestChain comment (Carl Dong) b8e95658d5909f93dfc7d1e6532661db8919e5b7 style-only: Make TestBlockValidity signature readable (Carl Dong) 0cdad753903640ff4240b715dec9d62f68e51407 validation: Use accessible chainstate in ChainstateManager::ProcessNewBlock (Carl Dong) ea4fed90219be17160136313c68c06d84176af08 validation: Use existing chainstate in ChainstateManager::ProcessNewBlockHeaders (Carl Dong) e0dc3057277c9576ddbfb8541599db0149e08bb6 validation: Move LoadExternalBlockFile to CChainState (Carl Dong) 5f8cd7b3a527999512161956db4d718688cb956f validation: Remove global ::ActivateBestChain (Carl Dong) 2a696472a1423e877bfa83f016f66c7e45be7369 validation: Pass in chainstate to ::NotifyHeaderTip (Carl Dong) 9c300cc8b3ce3d82874982fbf3087e48a6ac0ef2 validation: Pass in chainstate to TestBlockValidity (Carl Dong) 0e17c833cda67cdba5338bd7409061772b6d5edb validation: Make CChainState.m_blockman public (Carl Dong) d363d06bf7d6c3736140672ba8a7f82f4d6fb6ab validation: Pass in blockman to ContextualCheckBlockHeader (Carl Dong) f11d11600ddb0ddff6538250ae2a35df6112c3db validation: Move GetLastCheckpoint to BlockManager (Carl Dong) e4b95eefbc700ebc915bec312f77477ce3e87a7e validation: Move GetSpendHeight to BlockManager (Carl Dong) b026e318c39f59a06e29f1b25c7f577e01b25ccb validation: Move FindForkInGlobalIndex to BlockManager (Carl Dong) 3664a150ac7547c9336b571557af223d9e31aac9 validation: Remove global LookupBlockIndex (Carl Dong) eae54e6e60d7ed05b29d8345c0bb055397149ce8 scripted-diff: Use BlockManager::LookupBlockIndex (Carl Dong) 15d20f40e1321b24963b40c12958c7d30ad112ec validation: Move LookupBlockIndex to BlockManager (Carl Dong) f92dc6557a153b390a1ae1d0808ff7ed5d02c66e validation: Guard the active_chainstate with cs_main (Carl Dong) Pull request description: Overall PR: #20158 (tree-wide: De-globalize ChainstateManager) Note to reviewers: 1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so: 1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only** 2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase** 3. Remove `old_function` ACKs for top commit: jnewbery: utACK 67c9a83df19c6e2a2edb32336879204e7770b4a7 laanwj: re-ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7 ryanofsky: Code review ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7. Changes since last review: Tree-SHA512: 8744aba2dd57a40cd2fedca809b0fe24d771bc60da1bffde89601999384aa0df428057a86644a3f72fbeedbc8b04db6c4fd264ea0db2e73c279e5acc6d056cbf
2021-01-30fuzz: Avoid designated initialization (C++20) in fuzz testspracticalswift
2021-01-29refactor: remove boost::thread_group usagefanquake
2021-01-28validation: Move LoadExternalBlockFile to CChainStateCarl Dong
[META] This commit should be followed up by removing the comments and assertions meant only to show that the change is correct. LoadExternalBlockFile mainly acts on CChainState.
2021-01-28validation: Remove global ::ActivateBestChainCarl Dong
Instead use CChainState::ActivateBestChain, which is what the global one calls anyway.
2021-01-28scripted-diff: Use BlockManager::LookupBlockIndexCarl Dong
[META] In a previous commit, we moved ::LookupBlockIndex to become a member function of BlockManager. This commit is split out from that one since it can be expressed nicely as a scripted-diff. -BEGIN VERIFY SCRIPT- find_regex='LookupBlockIndex' \ && git grep -l -E "$find_regex" -- src \ | grep -v '^src/validation\.\(cpp\|h\)$' \ | xargs sed -i -E "s@${find_regex}@g_chainman.m_blockman.LookupBlockIndex@g" -END VERIFY SCRIPT-
2021-01-28Merge #20995: fuzz: Avoid initializing version to less than ↵MarcoFalke
MIN_PEER_PROTO_VERSION fad3d7625aa1c2b6c343946e709e87e7168f9d9d fuzz: Avoid initializing version to less than MIN_PEER_PROTO_VERSION (MarcoFalke) fa99e33aebed0109630474e11183b0726b410c2e fuzz: move-only FillNode implementation to cpp file (MarcoFalke) Pull request description: This fixes a fuzz bug introduced in #20881. Previously the nodes in the fuzz tests had their version initialized to a constant (`PROTOCOL_VERSION`). After #20881, the nodes have their version initialized to an arbitrary signed integer. This is problematic for several reasons: * Both `nVersion` and `m_greatest_common_version` may be initialized to `0`. If a `version` message is processed, this leads to a crash, because `m_greatest_common_version` must be `INIT_PROTO_VERSION` while the `version` message is processed. See #20138 * The "valid" range for `nVersion` is `[MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()]` (see check in net_processing) * The "valid" range for `m_greatest_common_version` is `std::min(nVersion, PROTOCOL_VERSION)` (see net_processing) Fix all issues by initializing `nVersion` and `m_greatest_common_version` to their valid ranges. ----- The crashers, if someone wants to try this at home: ``` ( echo 'dmVyc2lvbgAWFhYWFhYWFhYWFhYWFhYWFhYWFhZp/29uAPX//xYWFhYWFhYWFhYWFhYWFhYWFhYW FhYWFhYWaW9uAOr1//8WFhYWFha0ZXJzaW9uAPX//wAAAAAAABAAAAAAAAAAAAC0ZXJzaW9uAPX/ /wBPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT08AAAAAABAAAAAAAAAAAAAAAAAA AAAAAAAAAAAAAAAAAAAAAAAAAAAACgAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA AAAAAAAAAAAAAAB2ZXJzaW9uAACDJIO9vXYKAAAAAAAAAAAAAAAAAAAAAAB2ZfS1qmu1qhUVFWs=' | base64 --decode > /tmp/a ) && FUZZ=process_message_version ./src/test/fuzz/fuzz /tmp/a ``` ``` ( echo 'dmVyc2lvbgD//wAhTmiqN///NDcAAACENDL/iv//8DYAAHL///////79/RtcAJqamhqa/QEAAAD/ ///+/f1oZWFkZXJzAAAAAM8BAAAAIAYibkYRGgtZyq8SaGVhZGVycwAAAAD/NDcAAACENDL/iv// 8DYAAHL///////79/RtcAJqamhqa/QEAAAD////+/f1oZWFkZXJzAAAAAM8BAAAAIAYibkYRGgtZ yq8SaGVhZGVycwAAAADPAQAAACAGIm5GERoLWS1wb3J061u/KMNPOkwFXqZ///b5IgIAAD+5ubkb XD5hZGRyAJqamhqasP0BAAAAAAAAAP0BAAAAIf39/R0dHQAAAAAAMgAA///7//+gXqZ///b5IgIA AD+5ubm5ubm5AAAAAAAAAAAAAAAAAAAAAAAAAACAAAAAAAAAFgAAAAAAAAAAAAlBmv39/f1/f39B f39hZGRyAG5vAACaLgAdGzY2zwEAAAAgBiJuRhEaC1ktcG9ydOtbvyjDTzpMBV6mf//2+SICAAA/ ubm5G1w+YWRkcgCampoamrD9AQAAAAAAAAD9AQAAACH9/f0dHR0AAAAAADIAAP//+///oF6mf//2 +SICAAA/ubm5ubm5uQAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAAAAABYAAAAAAAAAAAAJQZr9/f39 f39/QX9/YWRkcgBubwAAmi4AHRs2NjY2NjY2NjYCAgI2NgIA/f39/f39Nv39/TUmABxc' | base64 --decode > /tmp/b ) && FUZZ=process_message_version ./src/test/fuzz/fuzz /tmp/b ``` ACKs for top commit: practicalswift: cr ACK fad3d7625aa1c2b6c343946e709e87e7168f9d9d Tree-SHA512: ea64ee99b94d8e619e3949d2d21252c1236412c0e40f44f2b73595ca70cd2da0bdab005fb1a54f65fb291e7b07fdd33577ce4a3a078ca933246b511ebcb0e52a
2021-01-26Merge bitcoin-core/gui#183: Add include for std::bind.MarcoFalke
2a39ccf1334ef3c48c6f9969a0fc916b9e10aae1 Add include for std::bind. (sinetek) Pull request description: Hi, this patch adds in <functional> because the GUI code makes use of std::bind. That's all. ACKs for top commit: jonasschnelli: utACK 2a39ccf1334ef3c48c6f9969a0fc916b9e10aae1 Tree-SHA512: fb5ac07d9cd5d006182b52857b289a9926362a2f1bfa4f7f1c78a088670e2ccf39ca28214781df82e8de3909fa3e69685fe1124a7e3ead758575839f5f2277a9
2021-01-26Merge #21000: fuzz: Add UBSan suppressions needed for fuzz tests to not warn ↵MarcoFalke
under -fsanitize=integer f0f8b1a076c362c6e26570a2129809f4d6a0abad fuzz: Add UBSan suppressions needed for fuzz tests to not warn under -fsanitize=integer (practicalswift) 58232e3ffb319d27b41cfe040cfea8e43a3fb7e9 fuzz: Avoid -fsanitize=integer warnings in fuzzing harnesses (practicalswift) Pull request description: Add UBSan suppressions needed for fuzz tests to not warn under `-fsanitize=integer`. Avoid `-fsanitize=integer` warnings in fuzzing harnesses. Suppressed warnings (excluding warnings from `src/crypto/` and `src/test/`): ``` addrman.cpp:306:24: runtime error: implicit conversion from type 'long' of value 5190149478 (64-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 895182182 (32-bit, unsigned) addrman.h:446:43: runtime error: implicit conversion from type 'int' of value -22 (32-bit, signed) to type 'const uint8_t' (aka 'const unsigned char') changed the value to 234 (8-bit, unsigned) arith_uint256.cpp:32:35: runtime error: left shift of 1712128 by 24 places cannot be represented in type 'uint32_t' (aka 'unsigned int') arith_uint256.cpp:47:39: runtime error: left shift of 4294966784 by 31 places cannot be represented in type 'uint32_t' (aka 'unsigned int') chain.cpp:151:12: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'unsigned long' changed the value to 18446744073709551615 (64-bit, unsigned) coins.cpp:114:22: runtime error: unsigned integer overflow: 0 - 96 cannot be represented in type 'unsigned long' compressor.cpp:162:33: runtime error: unsigned integer overflow: 15617702637291228364 * 10 cannot be represented in type 'unsigned long' compressor.cpp:188:11: runtime error: unsigned integer overflow: 2265760372865400000 * 10 cannot be represented in type 'unsigned long' hash.cpp:13:15: runtime error: left shift of 1692305888 by 15 places cannot be represented in type 'uint32_t' (aka 'unsigned int') pubkey.h:152:23: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'unsigned int' streams.h:570:31: runtime error: left shift of 350879 by 52 places cannot be represented in type 'uint64_t' (aka 'unsigned long') util/bip32.cpp:57:36: runtime error: left shift of 3241096244 by 1 places cannot be represented in type 'unsigned int' util/strencodings.cpp:562:38: runtime error: implicit conversion from type 'unsigned char' of value 255 (8-bit, unsigned) to type 'char' changed the value to -1 (8-bit, signed) util/strencodings.h:164:24: runtime error: implicit conversion from type 'int' of value -74 (32-bit, signed) to type 'unsigned long' changed the value to 18446744073709551542 (64-bit, unsigned) ``` The warnings above happen here: https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/addrman.cpp#L306 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/addrman.h#L446 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/arith_uint256.cpp#L32 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/arith_uint256.cpp#L47 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/chain.cpp#L151 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/coins.cpp#L114 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/compressor.cpp#L162 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/compressor.cpp#L188 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/hash.cpp#L13 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/pubkey.h#L152 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/streams.h#L570 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/util/bip32.cpp#L57 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/util/strencodings.cpp#L562 https://github.com/bitcoin/bitcoin/blob/32b191fb66e644c690c94cbfdae6ddbc754769d7/src/util/strencodings.h#L164 ACKs for top commit: MarcoFalke: review ACK f0f8b1a076c362c6e26570a2129809f4d6a0abad 🤚 Tree-SHA512: a8f04f7cc055d03653161de1d9d14d106a6280cea1e86a1243abcd57cf8e61dcf5f731d0ab0da5b390790e816022ff7a70759a641463bc7e3303076b8667009f
2021-01-26refactor: remove straggling boost::mutex usagefanquake
After the merge of #18710, the linter is warning: ```bash A new Boost dependency in the form of "boost/thread/mutex.hpp" appears to have been introduced: src/sync.cpp:#include <boost/thread/mutex.hpp> src/test/sync_tests.cpp:#include <boost/thread/mutex.hpp> ^---- failure generated from test/lint/lint-includes.sh ``` the interim #19337 was merged, which introduced more `boost::mutex` usage. Given we no longer use `boost::mutex`, just remove the double lock test and remaining includes.
2021-01-25fuzz: Avoid -fsanitize=integer warnings in fuzzing harnessespracticalswift
2021-01-25Merge #18710: Add local thread pool to CCheckQueueWladimir J. van der Laan
bb6fcc75d1ec94b733d1477c816351c50be5faf9 refactor: Drop boost::thread stuff in CCheckQueue (Hennadii Stepanov) 6784ac471bb32b6bb8e2de60986f123eb4990706 bench: Use CCheckQueue local thread pool (Hennadii Stepanov) dba30695fc42f45828db008e7e5b81cb2b5d8551 test: Use CCheckQueue local thread pool (Hennadii Stepanov) 01511776acb0c7ec216dc9c8112531067763f1cb Add local thread pool to CCheckQueue (Hennadii Stepanov) 0ef938685b5c079a6f5a98daf0e3865d718d817b refactor: Use member initializers in CCheckQueue (Hennadii Stepanov) Pull request description: This PR: - gets rid of `boost::thread_group` in the `CCheckQueue` class - allows thread safety annotation usage in the `CCheckQueue` class - is alternative to #14464 (https://github.com/bitcoin/bitcoin/pull/18710#issuecomment-616618525, https://github.com/bitcoin/bitcoin/pull/18710#issuecomment-617291612) Also, with this PR (I hope) it could be easier to resurrect a bunch of brilliant ideas from #9938. Related: #17307 ACKs for top commit: laanwj: Code review ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9 LarryRuane: ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9 jonatack: Code review ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9 and verified rebase to master builds cleanly with unit/functional tests green Tree-SHA512: fddeb720d5a391b48bb4c6fa58ed34ccc3f57862fdb8e641745c021841c8340e35c5126338271446cbd98f40bd5484f27926aa6c3e76fa478ba1efafe72e73c1
2021-01-23fuzz: Avoid initializing version to less than MIN_PEER_PROTO_VERSIONMarcoFalke
2021-01-23fuzz: move-only FillNode implementation to cpp fileMarcoFalke
This allows to modify the implementation without having to recompile all fuzz targets. Can be reviewed with --color-moved=dimmed-zebra
2021-01-21util: Add ArgsManager::GetCommand() and use it in bitcoin-walletMarcoFalke
Co-Authored-by: Anthony Towns <aj@erisian.com.au>
2021-01-21Merge #20932: refactor: Replace fs::absolute calls with AbsPathJoin callsMarcoFalke
da9caa1cedd69702aea44cb44b2fd0a2d6d56916 Replace fs::absolute calls with AbsPathJoin calls (Kiminuo) 66576c4fd532ac18b8b355ea93d25581a2c15654 test: Clear forced -walletdir setting after wallet init_tests (Kiminuo) Pull request description: This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17. This PR doesn't change behavior aside from adding an assert and fixing a test bug. ACKs for top commit: jonatack: Code review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916 only doxygen improvements since my last review per `git diff d867d7a da9caa1` MarcoFalke: review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916 📯 ryanofsky: Code review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916. Just comment and test tweaks since previous review. Tree-SHA512: c940ee60f3ba374d4927cf34cf12d27c4c735c94af591fbc0ca408c641b30f8f8fbcfe521d66bfbddf9877a1fc8cd99bd8a47ebcd2fa59789de6bd87a7b9cf4d
2021-01-21Merge #20946: fuzz: Consolidate fuzzing TestingSetup initializationMarcoFalke
abb6fa728598c4cc8874eae1c3c5e587e36424cd fuzz: Initialize a full TestingSetup where appropriate (Carl Dong) 713314abfa224efceb7ba6fa7fef37c449902936 fuzz: Consolidate fuzzing TestingSetup initialization (Carl Dong) Pull request description: ``` Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways: 1. Calling InitializeFuzzingContext, which implicitly constructs a static const BasicTestingSetup 2. Directly constructing a static const BasicTestingSetup in the initialize_* function 3. Directly constructing a static TestingSetup and reproducing the initialization arguments (I'm assuming because InitializeFuzzingContext only initializes a BasicTestingSetup) The new, relatively-simple MakeFuzzingContext function allows us to consolidate these methods of initialization by being flexible enough to be used in all situations. It: 1. Is templated so that we can choose to initialize any of the *TestingSetup classes 2. Has sane defaults which are often used in fuzzers but are also easily overridable 3. Returns a unique_ptr, explicitly transferring ownership to the caller to deal with according to its situation ``` ~~Question for fuzzing people: was it intentional that `src/test/fuzz/net.cpp` would directly instantiate the `BasicTestingSetup` and thus omit the `"-nodebuglogfile"` flag?~~ [Answered](https://github.com/bitcoin/bitcoin/pull/20946#issuecomment-761537108) ACKs for top commit: MarcoFalke: ACK abb6fa728598c4cc8874eae1c3c5e587e36424cd Tree-SHA512: 96a5ca6f4cd5ea0e9483b60165b31ae3e9003918c700a7f6ade48010f419f2a6312e10b816b3187f1d263798827571866e4c4ac0bbfb2e0c79dfad254cda68e7
2021-01-21fuzz: Initialize a full TestingSetup where appropriateCarl Dong
A full TestingSetup is required for both coins_view and load_external_block_file as they interact with the active chainstate.
2021-01-21fuzz: Consolidate fuzzing TestingSetup initializationCarl Dong
Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways: 1. Calling InitializeFuzzingContext, which implicitly constructs a static const BasicTestingSetup 2. Directly constructing a static const BasicTestingSetup in the initialize_* function 3. Directly constructing a static TestingSetup and reproducing the initialization arguments (I'm assuming because InitializeFuzzingContext only initializes a BasicTestingSetup) The new, relatively-simple MakeFuzzingContext function allows us to consolidate these methods of initialization by being flexible enough to be used in all situations. It: 1. Is templated so that we can choose to initialize any of the *TestingSetup classes 2. Has sane defaults which are often used in fuzzers but are also easily overridable 3. Returns a unique_ptr, explicitly transferring ownership to the caller to deal with according to its situation
2021-01-15Replace fs::absolute calls with AbsPathJoin callsKiminuo
This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17.
2021-01-15Merge #20908: fuzz: Use mocktime in process_message* fuzz targetsMarcoFalke
fa0a864b383a794910cdb215bb836dae12357bc6 fuzz: Use mocktime in process_message* fuzz targets (MarcoFalke) Pull request description: Use mocktime to allow time to advance deterministically during execution of a fuzz input. This also allows to drop the call to `JumpOutOfIbd`. ACKs for top commit: practicalswift: cr ACK fa0a864b383a794910cdb215bb836dae12357bc6 Tree-SHA512: e92fc70ec6bd49760173cb202549f364304e22b3f7127b9a4da8447cf9341008e477ad42c2599c2fde167bbcbc0e2d139709b4ef6371788bc2c1c3b7f589e11d
2021-01-14Merge #20828: fuzz: Introduce CallOneOf helper to replace switch-caseMarcoFalke
fa75d40ef866ef9ff8dc115e239ca6763aa23b06 fuzz: Introduce CallOneOf helper to replace switch-case (MarcoFalke) Pull request description: The current `switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, nn)) { case 0: ... case 1: ... case nn: ...` has several problems: * It makes it hard to review newly added targets, because it requires manual counting of cases * It makes it hard to update a target, because updating all case labels is trivial, but tedious to review and causes merge conflicts * ~~Updating the target raises the question whether the case labels should be preserved to not invalidate the existing fuzz inputs format. Fuzz input format might already change implicitly on every commit, so this isn't something worthwhile to pursue.~~ Edit: This pull doesn't fix this problem. Fix all issues by adding a new `CallOneOf` helper ACKs for top commit: ajtowns: ACK fa75d40ef866ef9ff8dc115e239ca6763aa23b06 - code review only jnewbery: utACK fa75d40ef866ef9ff8dc115e239ca6763aa23b06 Tree-SHA512: 2daa602b240b86c8e85a024e008f03a57ba60349377eed771f4d21a97a9dba9b66e93fff16ff1992018d4330be7a1a276944c3dfdf698748ce135626c380e563
2021-01-13Merge #20811: refactor: move net_processing implementation details out of headerWladimir J. van der Laan
c97f70c861ac6959b8116a9bca3031edeb2b2aaa net_processing: move Peer definition to .cpp (Anthony Towns) e0f2e6d2df7117a8dbf17c63c5149fc53a6fe2b2 net_processing: move PeerManagerImpl into cpp file (Anthony Towns) a568b82febb3ecbd5ebb7c3f9da27e762b0c68f6 net_processing: split PeerManager into interface and implementation classes (Anthony Towns) 0df3d3fd6bbbd0e06116797177ba797580553250 net_processing: make more of PeerManager private (Anthony Towns) 0d246a59b606c51728d10cb70004a6eedb951bca net, net_processing: move NetEventsInterface method docs to net.h (Anthony Towns) Pull request description: Moves the implementation details of `PeerManager` and all of `struct Peer` into net_processing.cpp. ACKs for top commit: jnewbery: ACK c97f70c861ac6959b8116a9bca3031edeb2b2aaa Sjors: ACK c97f70c861ac6959b8116a9bca3031edeb2b2aaa laanwj: Code review ACK c97f70c861ac6959b8116a9bca3031edeb2b2aaa vasild: ACK c97f70c861ac6959b8116a9bca3031edeb2b2aaa Tree-SHA512: 2e081e491d981c61bd78436a6a6c2eb41d3c2d86a1a8ef9d4d6f801b82cb64a8bf707a96db3429418d1704cffb60a657d1037a0336cbc8173fb79aef9eb72001
2021-01-13fuzz: Use mocktime in process_message* fuzz targetsMarcoFalke
2021-01-13Add include for std::bind.sinetek
2021-01-12Merge #20495: sync: Use decltype(auto) return type for WITH_LOCKfanquake
3eb94ec81b72b14f72a1f6ce5c9aa24476df755a sync: Use decltype(auto) return type for WITH_LOCK (Carl Dong) Pull request description: > Now that we're using C++17, we can use the decltype(auto) return type > for functions and lambda expressions. > > As demonstrated in this commit, this can simplify cases where previously > the compiler failed to deduce the correct return type. > > Just for reference, for the "assign to ref" cases fixed here, there are > 3 possible solutions: > > - Return a pointer and immediately deref as used before this commit > - Make sure the function/lambda returns declspec(auto) as used after > this commit > - Class& i = WITH_LOCK(..., return std::ref(...)); > > ----- > > References: > 1. https://en.cppreference.com/w/cpp/language/function#Return_type_deduction > 2. https://en.cppreference.com/w/cpp/language/template_argument_deduction#Other_contexts > 3. https://en.cppreference.com/w/cpp/language/auto > 4. https://en.cppreference.com/w/cpp/language/decltype > > Explanations: > 1. https://stackoverflow.com/a/21369192 > 2. https://stackoverflow.com/a/21369170 Thanks to sipa and ryanofsky for helping me understand this ACKs for top commit: jnewbery: utACK 3eb94ec81b72b14f72a1f6ce5c9aa24476df755a hebasto: ACK 3eb94ec81b72b14f72a1f6ce5c9aa24476df755a, I have reviewed the code and it looks OK, I agree it can be merged. I have verified possible warnings: ryanofsky: Code review ACK 3eb94ec81b72b14f72a1f6ce5c9aa24476df755a Tree-SHA512: 5f55c7722aeca8ea70e5c1a8db93e93ba0e356e8967e7f607ada38003df4b153d73c29bd2cea8d7ec1344720d37d857ea7dbfd2a88da1d92e0e9cbb9abd287df
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-10 10:35:00 +0000