aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-10-10Adjust RPCTypeCheckObj error stringLeonardo Araujo
2022-10-10Merge bitcoin/bitcoin#26254: iwyu: Add zmq source filesMacroFake
13afcc0cd4c2975852924d2d9be5e96096147716 iwyu: Add zmq source files (Hennadii Stepanov) Pull request description: ACKs for top commit: fanquake: ACK 13afcc0cd4c2975852924d2d9be5e96096147716 Tree-SHA512: 7af95e991fc2782aeba2edfef0a2f75f9c361058295586adb062087aa31c47cfcce2425aee9dd5153e18e018cf1f9272c9617c671b7262db55f241526c3fcb15
2022-10-10iwyu: Add zmq source filesHennadii Stepanov
2022-10-10Merge bitcoin/bitcoin#25322: build: Fix `capnp` package build for Androidfanquake
8b8edc25c13a3e613770bf38b21a2556192e6315 build: Specify native binaries explicitly when building `capnp` package (Hennadii Stepanov) a413595c37f51557f9506e0a279cd80fc9a6fb36 build: Fix `capnp` package build for Android (Hennadii Stepanov) Pull request description: On master (e3c08eb620a2f317fc09fdf20969fa26f02afb91): ``` $ make -C depends capnp MULTIPROCESS=1 HOST=aarch64-linux-android ANDROID_SDK=$ANDROID_HOME ANDROID_NDK=$ANDROID_HOME/ndk/23.2.8568313 ANDROID_API_LEVEL=28 ANDROID_TOOLCHAIN_BIN=$ANDROID_HOME/ndk/23.2.8568313/toolchains/llvm/prebuilt/linux-x86_64/bin ... ld: error: unable to find library -lkj ... ``` This PR fixes this error, and also improves configuring according to the docs. ACKs for top commit: ryanofsky: Code review ACK 8b8edc25c13a3e613770bf38b21a2556192e6315. I'd be a little curious to know what causes the error and how `--disable-shared` fixes it, but these changes all look good Tree-SHA512: 1b07b75f2a83932d8dc1f007e42a67d8327bd5fe4566f554dab4599e2a1e04b0144648790a1fd2ab1c295dba728586035aa0ebdbe5cf49df048ec87736895aaf
2022-10-10Merge bitcoin/bitcoin#26118: log: Use steady clock for bench loggingMacroFake
fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 Use steady clock for bench logging (MacroFake) faed342a2338d6a1a26cf977671a736662debae4 scripted-diff: Rename time symbols (MacroFake) Pull request description: Instead of using `0.001` and similar constants to "convert" an int64_t to milliseconds, use the type-safe `Ticks<>` helper. Also, use steady clock instead of system clock, since the durations are used for benchmarking. ACKs for top commit: fanquake: ACK fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 - validation bench output still looks sane. Tree-SHA512: e6525b5fdad6045ca500c56014897d7428ad288aaf375933d3b5939feddf257f6910d562eb66ebcde9186bef9a604ee8d763a318253838318d59df2a285be7c2
2022-10-10Merge bitcoin/bitcoin#26196: kernel: move RunCommandParseJSON to its own filefanquake
43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b refactor: move run_command from util to common (Cory Fields) 192325a77d593e404e74ef5e204aed8801b4e66f kernel: move RunCommandParseJSON to its own file (Cory Fields) Pull request description: Because libbitcoinkernel does not include this new object, this has the side-effect of eliminating its unnecessary `boost::process` dependency. This leaves libbitcoinkernel with 3 remaining boost dependencies: - `boost::date_time` for `util/time.cpp`, which I'll separate out next. Exactly like this PR. - `boost::signals2` for which I have a POC re-implementation here: https://github.com/theuni/bitcoin/commits/replace-boost-signals - `boost::multi_index` which I'm not sure about yet. ACKs for top commit: ryanofsky: Code review ACK 43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b. Could consider squashing the two commits, so the code just moves once instead of twice. fanquake: ACK 43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b Tree-SHA512: f2a46cac34aaadfb8a1442316152ad354f6990021b82c78d80cae9fd43cd026209ffd62132eaa99d5d0f8cf34e996b6737d318a9d9a3f1d2ff8d17d697abf26d
2022-10-10Merge bitcoin/bitcoin#26282: wallet: have prune error take precedence over ↵fanquake
assumedvalid 1c36bafc5f7db268546dcc86c793071a7e9d35e0 wallet: have prune error take precedence over assumedvalid (James O'Beirne) Pull request description: Fixes https://github.com/bitcoin/bitcoin/pull/23997#discussion_r891412739. From Russ Yanofsky: > Agree with all of Marco's points here and think this should be updated > > If havePrune and hasAssumedValidChain are both true, better to show havePrune error message. Assumed-valid error message is vague and not very actionable. Would suggest "Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order though assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height {block_height}" ACKs for top commit: MarcoFalke: ACK 1c36bafc5f7db268546dcc86c793071a7e9d35e0 aureleoules: ACK 1c36bafc5f7db268546dcc86c793071a7e9d35e0 Tree-SHA512: bfb0024bb962525cbbd392ade3c0331a8b0525e7f2f2ab52b2dbb9b6dd6311070d85ecb762a7689db84a30991971865698ab6fec187206e6a92133790c5a91dc
2022-10-10Merge bitcoin/bitcoin#25073: test: Cleanup miner_testsfanquake
faa15527d7e0c7923ff9c0fad7bab4648705ed0f test: Use dedicated mempool in TestBasicMining (MacroFake) fafab384a0a5f6d80195307b7bbeb00515da432b test: Use dedicated mempool in TestPackageSelection (MacroFake) fa4055d79c7ea1d4c3b694e39cafa98a1c7ba8bb test: Use dedicated mempool in TestPrioritisedMining (MacroFake) fa2921828511816d0420c567386e1da0391b3ad7 test: Pass mempool reference to AssemblerForTest (MacroFake) Pull request description: This cleans up the miner tests: * Removes duplicate/redundant and thus confusing chainparams object. * Uses a fresh mempool for each subtest instead of using the "global" one from the testing setup. This makes it easier to follow the tests in smaller scopes. Also it makes sure the mempool is truly cleared by reconstructing it. Finally, this removes calls to `clear`, see https://github.com/bitcoin/bitcoin/pull/19909 ACKs for top commit: glozow: utACK faa15527d7e0c7923ff9c0fad7bab4648705ed0f Tree-SHA512: ced1260f6ab70fba74b0fac7ff4fc7adfddcd2f3bee785249d2a4a9055ac253eff9090edbda7a17e72a71a81b56ff708d5ff64e1f57ebc7b7747d6c88fec51e3
2022-10-10Merge bitcoin/bitcoin#26284: Fix comment typosMacroFake
adb1714426365fb72f73097610ba217ac94ea560 Fix comment typos in scriptpubkeyman.cpp, wallet.cpp, wallet.h (Dimitris Tsapakidis) Pull request description: Fixes a number of comment typos found in the code. Top commit has no ACKs. Tree-SHA512: c2c996b66d33ecf0ee734b76303a0f2444e184d2f3ff6931768712ca51011ad51e54336c33a2ff55133766d20ae6adcbb14ddc754dde58b1fe9167d68f54fec5
2022-10-10Merge bitcoin/bitcoin#26183: build: split ARM crc & crypto extension checksfanquake
20adaeaef5fa190c6c996e010683e9728b947ac9 build: split ARM crc & crypto extension checks (fanquake) Pull request description: We currently perform the same check twice, to put the same set of flags in two different variables. Split the checks so we test for the `crc` and `crypto` extensions independently. If we don't want to split, we should just delete the second `AX_CHECK_COMPILE_FLAG` check, and set `ARM_CRC_CXXFLAGS` & `ARM_SHANI_CXXFLAGS` at the same time. Guix Build: ```bash 045392a6a4f538723b7759c67eeafd832735de7294b72b3a7f488d05a13711f7 guix-build-20adaeaef5fa/output/aarch64-linux-gnu/SHA256SUMS.part 054fda86577d757788a1c87508268402535fcbe869240309a2c91997234389cf guix-build-20adaeaef5fa/output/aarch64-linux-gnu/bitcoin-20adaeaef5fa-aarch64-linux-gnu-debug.tar.gz 92dc2513b2b6d87c0869ae18493fd9d0e2690b5b02bfd4310d54f4d394cfccdf guix-build-20adaeaef5fa/output/aarch64-linux-gnu/bitcoin-20adaeaef5fa-aarch64-linux-gnu.tar.gz 2515cfc708cc6ce0e650ca00c49de8dad856b54741ddc0c195845fc6ce2d67db guix-build-20adaeaef5fa/output/arm-linux-gnueabihf/SHA256SUMS.part fa0a956365e62b484f66dcf9763a02858db5c7e99317861819a54a15589ced80 guix-build-20adaeaef5fa/output/arm-linux-gnueabihf/bitcoin-20adaeaef5fa-arm-linux-gnueabihf-debug.tar.gz 1b3ddf2b1bbdc7632696ca78908e69b4fd156ccf7afa8078b5541d2ac10ab931 guix-build-20adaeaef5fa/output/arm-linux-gnueabihf/bitcoin-20adaeaef5fa-arm-linux-gnueabihf.tar.gz f87d8e23df60b208a631f6642f6c2cc0fc8e4e5e9563b36d1de9d371f22a69d9 guix-build-20adaeaef5fa/output/arm64-apple-darwin/SHA256SUMS.part c24ac07bfa935fd40358823d95ef01128a03b80deec6b2cb8bed122994e8adc2 guix-build-20adaeaef5fa/output/arm64-apple-darwin/bitcoin-20adaeaef5fa-arm64-apple-darwin-unsigned.dmg 696660e030accadc27901dfb4e120aa2fefefa8cc2a33ae887e3c98e5d4795f5 guix-build-20adaeaef5fa/output/arm64-apple-darwin/bitcoin-20adaeaef5fa-arm64-apple-darwin-unsigned.tar.gz 30dcd3f543781ac0e07e36336c2901a25a0829e0d1425c25b3c7aba1d0e5420e guix-build-20adaeaef5fa/output/arm64-apple-darwin/bitcoin-20adaeaef5fa-arm64-apple-darwin.tar.gz 4d63db45f28fcb99aa8f3b30cf06afef80dd308a8d2fdf874752accb3f341258 guix-build-20adaeaef5fa/output/dist-archive/bitcoin-20adaeaef5fa.tar.gz eb208b98b3118e9f8240aab91c7ecb2f9b778109bc19d81d0ba73b3e35aa1123 guix-build-20adaeaef5fa/output/powerpc64-linux-gnu/SHA256SUMS.part 8b0de7008b1932ed18d3ab71ca309dc4919096e226e0a7197bd192e1ba96da82 guix-build-20adaeaef5fa/output/powerpc64-linux-gnu/bitcoin-20adaeaef5fa-powerpc64-linux-gnu-debug.tar.gz bcbc269cc4b5883397c516ef3ef6df564f4a81c240d5afcf912a2bf9554ff148 guix-build-20adaeaef5fa/output/powerpc64-linux-gnu/bitcoin-20adaeaef5fa-powerpc64-linux-gnu.tar.gz e5f7fd823056449a495a68d18fe941b472479bc59d9d4d11a041a4e2cc4044ec guix-build-20adaeaef5fa/output/powerpc64le-linux-gnu/SHA256SUMS.part 73ee7e786372b32ab840f0c00ca0479ddd022b3d37219cd929cb49f744c174e3 guix-build-20adaeaef5fa/output/powerpc64le-linux-gnu/bitcoin-20adaeaef5fa-powerpc64le-linux-gnu-debug.tar.gz 08f64c9aae4d9beef88d8fbae8ad0152517de74bedc88540775c4f757c8b6b9a guix-build-20adaeaef5fa/output/powerpc64le-linux-gnu/bitcoin-20adaeaef5fa-powerpc64le-linux-gnu.tar.gz fe3c28fdb1ee9d5b6ca3ba4510d61c052567edb3b93fdde929ed197072c0fd66 guix-build-20adaeaef5fa/output/riscv64-linux-gnu/SHA256SUMS.part 890d6b96edcc431620eede6239dec51368aff917010e03dabeb29d6a672d7a28 guix-build-20adaeaef5fa/output/riscv64-linux-gnu/bitcoin-20adaeaef5fa-riscv64-linux-gnu-debug.tar.gz df1fc0c9af4799cfe170444e21965f2a600aa193fdd0da542fedceeb3081b194 guix-build-20adaeaef5fa/output/riscv64-linux-gnu/bitcoin-20adaeaef5fa-riscv64-linux-gnu.tar.gz f69cae0b2d0eadb336cf314a888b1e0bed241f38954fe58ca9c9c2d00e49b74e guix-build-20adaeaef5fa/output/x86_64-apple-darwin/SHA256SUMS.part acc5fa9725bba738d10bb8b1e7df2d8a7b0e648015e1c046f67451d343f68224 guix-build-20adaeaef5fa/output/x86_64-apple-darwin/bitcoin-20adaeaef5fa-x86_64-apple-darwin-unsigned.dmg 7e4d8cb6d74434ba9084f487187c49cd5a4138c9ae03a6c2236cdffadb236bc8 guix-build-20adaeaef5fa/output/x86_64-apple-darwin/bitcoin-20adaeaef5fa-x86_64-apple-darwin-unsigned.tar.gz 8d93add28b20dfb2a556d3867cfbf218db336d7eefee6ab6f76a1bb4dd4ae20b guix-build-20adaeaef5fa/output/x86_64-apple-darwin/bitcoin-20adaeaef5fa-x86_64-apple-darwin.tar.gz ba0863eda963db706d2880daa8bc526e6332097010fa7227f513a2d715b6cd6c guix-build-20adaeaef5fa/output/x86_64-linux-gnu/SHA256SUMS.part 6915794f3cdc8ad9b305b6baa58f89f7493097b88c0af190d52d93457a17e8d8 guix-build-20adaeaef5fa/output/x86_64-linux-gnu/bitcoin-20adaeaef5fa-x86_64-linux-gnu-debug.tar.gz 467b05298058ec507c3b247c423f3ea7e027ecf62e45d7ae4b81160118bc0d02 guix-build-20adaeaef5fa/output/x86_64-linux-gnu/bitcoin-20adaeaef5fa-x86_64-linux-gnu.tar.gz 51a534803deaabcbba27d82359ef46e4d5b9e7b121ab71e1975c2a0d1c4c6f45 guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/SHA256SUMS.part c9c5496f20bac01ed6439746aff9ca3dd55708718902c898e99f3d5741b167a3 guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/bitcoin-20adaeaef5fa-win64-debug.zip cfaac54be36789927f83172c0af44c50648f63df7cdc9d81774a170e5ab6e3e5 guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/bitcoin-20adaeaef5fa-win64-setup-unsigned.exe 759b79660c291dcc7da88088de3bb666162fed5c9d94bb24f10cef6e781c565f guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/bitcoin-20adaeaef5fa-win64-unsigned.tar.gz f0124333d384ff6962e2131c7b2814bf5c968e77b63ff1b2c7d19cb4c571757c guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/bitcoin-20adaeaef5fa-win64.zip ``` ACKs for top commit: jarolrod: ACK 20adaeaef5fa190c6c996e010683e9728b947ac9 Tree-SHA512: 8b515b95ba4d41ca2ce91448339841dcfb80feb028e9e3bc67a72e72d93669e1257534c11286489a60ae240f6ad6e68f56615818fefd1c09a07a1bee4976fa6e
2022-10-10Merge bitcoin/bitcoin#26277: test: Remove confusing DUMMY_P2WPKH_SCRIPTfanquake
fa8a305ddde15abc0df03a3af0a03b92405d68f6 test: Remove confusing DUMMY_P2WPKH_SCRIPT (MacroFake) Pull request description: It is confusing because, it is *not* a P2WPKH script, and it is nonstandard. See also https://github.com/bitcoin/bitcoin/pull/26265/files#r989827855 Fix all issues by removing it, and also remove the no longer needed `-acceptnonstdtxn` setting from the test. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/26277/commits/fa8a305ddde15abc0df03a3af0a03b92405d68f6 theStack: Code-review ACK fa8a305ddde15abc0df03a3af0a03b92405d68f6 📜 Tree-SHA512: 64f3e0009b055e4fd4428b20f3e85582e1608e9b06e500b8fbfeb91fc35ce510e69d051e8f48ce35d0320067793e12f4423b214cc1f68c217a5872e0ad97d211
2022-10-10Merge bitcoin/bitcoin#26215: index: Improve ↵fanquake
BaseIndex::BlockUntilSyncedToCurrentChain reliability 8891949bdcb25093d3a6703ae8228c3c3687d3a4 index: Improve BaseIndex::BlockUntilSyncedToCurrentChain reliability (Ryan Ofsky) Pull request description: Since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb from PR https://github.com/bitcoin/bitcoin/pull/21726, index `BlockUntilSyncedToCurrentChain` behavior has been less reliable, and there has also been a race condition in the `coinstatsindex_initial_sync` unit test. It seems better for `BlockUntilSyncedToCurrentChain` to actually wait for the last connected block to be fully processed, than to be able to return before prune locks are set, so this switches the order of `m_best_block_index = block;` and `UpdatePruneLock` statements in `SetBestBlockIndex` to make it more reliable. Also since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb, there has been a race condition in the `coinstatsindex_initial_sync` test. Before that commit, the atomic index best block pointer `m_best_block_index` was updated as the last step of `BaseIndex::BlockConnected`, so `BlockUntilSyncedToCurrentChain` could safely be used in tests to wait for the last `BlockConnected` notification to be finished before stopping and destroying the index. But after that commit, calling `BlockUntilSyncedToCurrentChain` is no longer sufficient, and there is a race between the test shutdown code which destroys the index object and the new code introduced in that commit calling `AllowPrune()` and `GetName()` on the index object. Reproducibility instructions for this are in https://github.com/bitcoin/bitcoin/issues/25365#issuecomment-1259744133 This commit fixes the `coinstatsindex_initial_sync` race condition, even though it will require an additional change to silence TSAN false positives, https://github.com/bitcoin/bitcoin/pull/26188, after it is fixed. So this partially addresses but does not resolve the bug reporting TSAN errors https://github.com/bitcoin/bitcoin/issues/25365. There is no known race condition outside of test code currently, because the bitcoind `Shutdown` function calls `FlushBackgroundCallbacks` not `BlockUntilSyncedToCurrentChain` to safely shut down. Co-authored-by: vasild Co-authored-by: MarcoFalke ACKs for top commit: mzumsande: re-ACK 8891949bdcb25093d3a6703ae8228c3c3687d3a4 Tree-SHA512: 52e29e3772a0c92873c54e5ffb31dd66a909b68a2031b7585713cd1d976811289c98bd9bb41679a8689062f03be4f97bb8368696e789caa4607c2fd8b1fe289b
2022-10-10Merge bitcoin/bitcoin#26258: refactor: Remove unused CDataStream::rdbuf methodfanquake
fabbbe32ee09430d356fd1843f7d5c716b5f46cc Remove unused CDataStream::rdbuf method (MacroFake) Pull request description: It is unused and seems unlikely to be ever used. ACKs for top commit: theStack: Code-review ACK fabbbe32ee09430d356fd1843f7d5c716b5f46cc aureleoules: ACK fabbbe32ee09430d356fd1843f7d5c716b5f46cc Tree-SHA512: 5804642658f96a0fb51482ebf3a062bb0f997c1e0527455afa4aceeeb6c1ad139a98b14a7c8a0909daba733a83bdc24fcadad45060ead4be6eb3dc3e66c129e2
2022-10-09Merge bitcoin/bitcoin#26103: refactor: mempool: use CTxMemPool::Limitsglozow
33b12e5df6aa14344dd084e0c6f2d34158fca383 docs: improve docs where MemPoolLimits is used (stickies-v) 6945853c0bf3b1941bfd18b5ffbd5a81be0e56da test: use NoLimits() in MempoolIndexingTest (stickies-v) 3a86f24a4c1e4e985b1d90eddc135b8dd17049a4 refactor: mempool: use CTxMempool::Limits (stickies-v) b85af25f8770974bae4ef3fae64e75ef6dd2d3c2 refactor: mempool: add MemPoolLimits::NoLimits() (stickies-v) Pull request description: Mempool currently considers 4 limits regarding ancestor and descendant count and size, which get passed around between functions quite a bit. This PR uses `CTxMemPool::Limits` introduced in https://github.com/bitcoin/bitcoin/pull/25290 to simplify those signatures and callsites. The purpose of this PR is to improve readability and maintenance, without behaviour change. As noted in the first commit "refactor: mempool: change MemPoolLimits members to uint", we currently have an underflow issue where a user could pass a negative `-limitancestorsize`, which is eventually cast to an unsigned integer. This behaviour already exists. Because it's orthogonal and to minimize scope, I think this should be fixed in a separate PR. ACKs for top commit: hebasto: ACK 33b12e5df6aa14344dd084e0c6f2d34158fca383, I have reviewed the code and it looks OK, I agree it can be merged. glozow: reACK 33b12e5df6aa14344dd084e0c6f2d34158fca383 Tree-SHA512: 591c6dcee1894f1c3ca28b34a680eeadcf0d40cda92451b4a422c03087b27d682b5e30ba4367abd75a99b5ccb115b7884b0026958d3c7dddab030549db5a4056
2022-10-09Fix comment typos in scriptpubkeyman.cpp, wallet.cpp, wallet.hDimitris Tsapakidis
Fix comment typos: sigature -> signature ponter -> pointer it's key -> its key
2022-10-09Merge bitcoin/bitcoin#26281: docs: fix m_children to be a member of ↵glozow
CTxMemPoolEntry 01bf4af4f2e67420b0180ffd39098758b1fc5e1b docs: fix m_children to be a member of CTxMemPoolEntry (stickies-v) Pull request description: Small documentation fix to reflect that `m_children` [is a member](https://github.com/bitcoin/bitcoin/blob/73b61717a977fc9d23f1bae3f8620641a9dee1f3/src/txmempool.h#L99) of `CTxMemPoolEntry`, not `CTxMemPool` ACKs for top commit: hebasto: ACK 01bf4af4f2e67420b0180ffd39098758b1fc5e1b, wrong wording was introduced in bitcoin/bitcoin#19478. glozow: ACK 01bf4af4f2 Tree-SHA512: b66c43b92fda44682b1f67c43073ca9e133a6dc03cd28253e571e67170531138c20b22ffdb08f312fb2d47a1f869b876611646b54325c8b614d12049befad578
2022-10-07wallet: have prune error take precedence over assumedvalidJames O'Beirne
From Russ Yanofsky: "Agree with all of Marco's points here and think this should be updated If havePrune and hasAssumedValidChain are both true, better to show havePrune error message. Assumed-valid error message is vague and not very actionable. Would suggest "Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order though assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height {block_height}" Co-authored-by: MacroFake <MarcoFalke@gmail.com> Co-authored-by: Russ Yanofsky <russ@yanofsky.org>
2022-10-07docs: fix m_children to be a member of CTxMemPoolEntrystickies-v
2022-10-07test: Remove confusing DUMMY_P2WPKH_SCRIPTMacroFake
2022-10-06Merge bitcoin/bitcoin#26272: test: Prevent UB in `minisketch_tests.cpp`MacroFake
97007e2b9b7a578315df2b1549cd6075130e8f05 test: Prevent UB in `minisketch_tests.cpp` (Hennadii Stepanov) Pull request description: [`std::optional::operator*`](https://en.cppreference.com/w/cpp/utility/optional/operator*), which follows after the changed line, can cause UB. This PR addresses https://github.com/bitcoin/bitcoin/issues/26262#issuecomment-1268855418 ACKs for top commit: stickies-v: ACK 97007e2b9b7a578315df2b1549cd6075130e8f05 Tree-SHA512: a7dde8dac0cbdfa362fa1158b4564eccff9405852612227d581690c9a34084b3467ae6d4c0269262688d75339dcea90aaa38fccbba9be92d2643c2113860f3d6
2022-10-06test: Prevent UB in `minisketch_tests.cpp`Hennadii Stepanov
2022-10-06Merge bitcoin/bitcoin#24364: refactor: remove duplicate code from BlockAssemblerglozow
0f40d653218789aa176ca2f844e3222d2ad890a3 refactor: remove duplicate code from BlockAssembler (James O'Beirne) Pull request description: Found while reminding myself how transactions are chosen for blocks. Take it or leave it! ACKs for top commit: glozow: ACK 0f40d653218789aa176ca2f844e3222d2ad890a3 theStack: Concept and code-review ACK 0f40d653218789aa176ca2f844e3222d2ad890a3 Tree-SHA512: 8a2694e670ce3fe897ab8f64f64c8df5f8487fc1264527a3abbcba0e5b921fb693416497ccd62508295bc33f202c65556b91b6af463acb91aab43138d2492c14
2022-10-05index: Improve BaseIndex::BlockUntilSyncedToCurrentChain reliabilityRyan Ofsky
Since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb from PR https://github.com/bitcoin/bitcoin/pull/21726, index `BlockUntilSyncedToCurrentChain` behavior has been less reliable, and there has also been a race condition in the `coinstatsindex_initial_sync` unit test. It seems better for `BlockUntilSyncedToCurrentChain` to actually wait for the last connected block to be fully processed, than to be able to return before prune locks are set, so this switches the order of `m_best_block_index = block;` and `UpdatePruneLock` statements in `SetBestBlockIndex` to make it more reliable. Also since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb, there has been a race condition in the `coinstatsindex_initial_sync` test. Before that commit, the atomic index best block pointer `m_best_block_index` was updated as the last step of `BaseIndex::BlockConnected`, so `BlockUntilSyncedToCurrentChain` could safely be used in tests to wait for the last `BlockConnected` notification to be finished before stopping and destroying the index. But after that commit, calling `BlockUntilSyncedToCurrentChain` is no longer sufficient, and there is a race between the test shutdown code which destroys the index object and the new code introduced in that commit calling `AllowPrune()` and `GetName()` on the index object. Reproducibility instructions for this are in https://github.com/bitcoin/bitcoin/issues/25365#issuecomment-1259744133 This commit fixes the `coinstatsindex_initial_sync` race condition, even though it will require an additional change to silence TSAN false positives, https://github.com/bitcoin/bitcoin/pull/26188, after it is fixed. So this partially addresses but does not resolve the bug reporting TSAN errors https://github.com/bitcoin/bitcoin/issues/25365. There is no known race condition outside of test code currently, because the bitcoind `Shutdown` function calls `FlushBackgroundCallbacks` not `BlockUntilSyncedToCurrentChain` to safely shut down. Co-authored-by: Vasil Dimov <vd@FreeBSD.org> Co-authored-by: MacroFake <falke.marco@gmail.com>
2022-10-05Merge bitcoin/bitcoin#26252: refactor: Make 64-bit shift explicitMacroFake
5c5b85d0e7e7bb6eea47be60e20140b9fa9fa745 refactor: Make 64-bit shift explicit (Hennadii Stepanov) Pull request description: [`std::array::at()`](https://en.cppreference.com/w/cpp/container/array/at) expects an argument of the `size_t` type. This PR avoids implicit type conversion (for both 64-bit and 32-bit systems). Also it enables MSVC warning [C4334](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334) for all codebase. ACKs for top commit: MarcoFalke: ACK 5c5b85d0e7e7bb6eea47be60e20140b9fa9fa745 🚎 jonatack: Code review ACK 5c5b85d0e7e7bb6eea47be60e20140b9fa9fa745 Tree-SHA512: fda850a42068f2ada9f877fac9ff8af1e22b5dcb3e708f5b95c316e77c52c72d33cd9ec6507a7f5d1731d1afdf5af6dc65025d388cc480f82c46f4d88ef2d306
2022-10-05Remove unused CDataStream::rdbuf methodMacroFake
It is unused and seems unlikely to be ever used.
2022-10-05docs: improve docs where MemPoolLimits is usedstickies-v
2022-10-05test: use NoLimits() in MempoolIndexingTeststickies-v
The (100, 1000000, 1000, 1000000) limits are arbitrarily high and don't restrict anything, they are just meant to calculate ancestors properly. Using NoLimits() makes this intent more clear and simplifies the code.
2022-10-05refactor: mempool: use CTxMempool::Limitsstickies-v
Simplifies function signatures by removing repetition of all the ancestor/descendant limits, and increases readability by being more verbose by naming the limits, while still reducing the LoC.
2022-10-05refactor: mempool: add MemPoolLimits::NoLimits()stickies-v
There are quite a few places in the codebase that require us to construct a CTxMemPool without limits on ancestors and descendants. This helper function allows us to get rid of all that duplication.
2022-10-05test: Use dedicated mempool in TestBasicMiningMacroFake
No need for a shared mempool. Also remove unused chainparams parameter. Can be reviewed with --ignore-all-space
2022-10-05test: Use dedicated mempool in TestPackageSelectionMacroFake
No need for a shared mempool. Also remove unused chainparams parameter.
2022-10-05test: Use dedicated mempool in TestPrioritisedMiningMacroFake
No need for a shared mempool. Also remove unused chainparams parameter.
2022-10-05test: Pass mempool reference to AssemblerForTestMacroFake
2022-10-05Merge bitcoin/bitcoin#26256: ci: Remove clang-format from lint taskMacroFake
fa0437655417a9179a25e7717fdc1506f915bb08 Remove clang-format from lint task (MacroFake) Pull request description: clang-format could be used in scripted diffs, but remained largely unused. So remove the install bloat, as it is unlikely to be used in the future. ACKs for top commit: fanquake: ACK fa0437655417a9179a25e7717fdc1506f915bb08 hebasto: ACK fa0437655417a9179a25e7717fdc1506f915bb08 Tree-SHA512: e0a3ee47d2aa2565dd34676914c558c985eaeb522a05f10bcaac115871edcf0d7f101b517e4d452ca5223c40b18ad02883c31e2da3d1f4ff86464a9af0097b11
2022-10-05Remove clang-format from lint taskMacroFake
clang-format could be used in scripted diffs, but remained largely unused.
2022-10-05Merge bitcoin/bitcoin#26250: fuzz: add mempool_utils.cppMacroFake
8a6b6dfcd8d26b62c3a13beba72440635fcc5e19 fuzz: pass max fee into ConsumeTxMemPoolEntry (fanquake) eb155692804b4278f6638c74273c1d9d35cd6ab7 fuzz: add util/mempool/h.cpp (fanquake) Pull request description: Moving the heavy (Boost) mempool code out of fuzz/util.h. Means that (for ex) a crypto_common fuzz unit doesn't need to care about seeing endless Boost headers. This results in a ~10% speedup (for me) when compiling the fuzz tests. Your results may vary. ACKs for top commit: MarcoFalke: review ACK 8a6b6dfcd8d26b62c3a13beba72440635fcc5e19 🍮 Tree-SHA512: 27dc9d9581ac0b1b319cc0dc08fe5f8fbf9269386a5cb23f6fd5d8231bf015ed942ab4414d8001220541be0013756354578ddab1fec607c6fba04daf421bc870
2022-10-04refactor: move run_command from util to commonCory Fields
Quoting ryanofsky: "util can be the library for things included in the kernel which the kernel can depend on, and common can be the library for other code that needs to be shared internally, but should not be part of the kernel or shared externally."
2022-10-04refactor: Make 64-bit shift explicitHennadii Stepanov
Also this change enables MSVC warning C4334 for all codebase. https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334
2022-10-04Merge bitcoin/bitcoin#26234: ci: Allow PIP_PACKAGES on centosfanquake
fa6054e952f4522b98dc89609033950a3cbfd06c ci: Allow PIP_PACKAGES on centos (MacroFake) fac085a05cc518b14271353128bb1fa830b3c612 ci: Remove unused package (MacroFake) Pull request description: This was added in 7fc5e865b93af59364e9c8bf75ec68b4decc7e5d but I can't see a reason why this should be forbidden. This is also needed for other changes (bumping the minimum python version). ACKs for top commit: hebasto: re-ACK fa6054e952f4522b98dc89609033950a3cbfd06c Tree-SHA512: e8ead9ee00079024eb1e8c6e7b31c78cf2a3392159b444765c2ea9a58bed2a7550bf71083210692a45bb8ed7896cb882b72bf70baa13a2384864b2b510b73005
2022-10-04Merge bitcoin/bitcoin#26243: test: Remove unused fCheckpointsEnabled from ↵fanquake
miner_tests fa9436e90859c31d9aa7bc0b7f223ab93ec14bdc test: Remove unused fCheckpointsEnabled from miner_tests (MacroFake) Pull request description: The earliest checkpoint is at height 11111, so this can't possibly have any impact on this test. ACKs for top commit: fanquake: ACK fa9436e90859c31d9aa7bc0b7f223ab93ec14bdc - given the low number of blocks, having the additional check in `ContextualCheckBlockHeader()` enabled should be a no-op, so disabling and re-enabling is dead code. Tree-SHA512: 7d1b952c297c915e9588761f82f5006cf5186b7ff30e8a1c702302e0b44afe536bde9eda8acf2995825ae01d2ad9d2393ae2feefb29f15676aaf71881941579b
2022-10-04fuzz: pass max fee into ConsumeTxMemPoolEntryfanquake
2022-10-04fuzz: add util/mempool/h.cppfanquake
Moving the mempool code (Boost) out of util.h, results in a ~10% speedup (for me) when compiling the fuzz tests.
2022-10-04Merge bitcoin/bitcoin#26244: build, msvc: Enable C4834 warningfanquake
f3e40c481ad77c9664198d4ad89c2fc4e6bfc10f build, msvc: Enable C4834 warning (Hennadii Stepanov) Pull request description: Since bitcoin/bitcoin#26189 our codebase is C4834 warning free. See https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c4834. ACKs for top commit: sipsorcery: tACK f3e40c481ad77c9664198d4ad89c2fc4e6bfc10f. Tree-SHA512: 2dd8fdb5d6c0d4eaf7ffec82039411963010ecea559adfe7e2b5587f467135bd665e0cef5c748e4d6d9ba3c2038d6a5c79980972884adbaaac3bda3b2fbbc408
2022-10-04Merge bitcoin/bitcoin#26249: ci: Workaround Windows filesystem executable ↵MacroFake
bit loss b8d361ab6fa2c672a08114e20a29a12537148e2d ci: Workaround Windows filesystem executable bit loss (Hennadii Stepanov) Pull request description: Fixes https://cirrus-ci.com/task/5946581265416192: ``` From https://github.com/bitcoin/bitcoin * branch refs/pull/26103/merge -> FETCH_HEAD error: Your local changes to the following files would be overwritten by checkout: ci/lint/04_install.sh ci/lint/06_script.sh contrib/devtools/gen-manpages.py contrib/devtools/symbol-check.py contrib/signet/getcoins.py contrib/signet/miner test/functional/feature_proxy.py test/functional/feature_taproot.py test/functional/interface_rest.py test/functional/mining_prioritisetransaction.py test/functional/rpc_blockchain.py test/functional/rpc_fundrawtransaction.py test/functional/rpc_help.py test/functional/rpc_rawtransaction.py test/functional/test_framework/test_framework.py test/functional/test_runner.py test/functional/wallet_basic.py test/functional/wallet_bumpfee.py test/functional/wallet_hd.py test/functional/wallet_importmulti.py test/functional/wallet_listdescriptors.py test/functional/wallet_multiwallet.py test/functional/wallet_resendwallettransactions.py test/functional/wallet_sendall.py test/lint/lint-locale-dependence.py Please commit your changes or stash them before you switch branches. Aborting ``` Top commit has no ACKs. Tree-SHA512: 2b33d882a515bb17c7c2ae8cfe73541483cdc15e736909afaf42befc8f648dba5dc83ff58ebd6d38a5650a8eca01907ae6c61537927ac9718bd9582d8501647d
2022-10-04ci: Workaround Windows filesystem executable bit lossHennadii Stepanov
2022-10-04kernel: move RunCommandParseJSON to its own fileCory Fields
Because libbitcoinkernel does not include this new object, this has the side-effect of eliminating the unnecessary boost::process dependency.
2022-10-04Merge bitcoin/bitcoin#26236: ci: Use same `merge_script` implementation for ↵fanquake
Windows as for all 37cf4720635b63cbe36a900a2411718704b63899 ci: Use same `merge_script` implementation for Windows as for all (Hennadii Stepanov) ac1d99240af6c5d3ed5db2beea1479903d949a37 ci: Move `git config` commands into script where they are used (Hennadii Stepanov) Pull request description: This PR is a continuation of bitcoin/bitcoin#26202 and it suggests the same approach for the "Win64 native" CI task. Top commit has no ACKs. Tree-SHA512: 3154c9f30bc62549d738dc337e24f66614420417c349770c8381cc29f825f75695c9abbbb8dc57abbfda1375bf353f7c68e1a3766fd6d2440792e2d7fb68e15e
2022-10-04Merge bitcoin/bitcoin#26128: doc: add missing historical release notesfanquake
cb075d245e3a9106783f5edd0ed2cf1f7e43cdf8 doc: add historical 0.21.2 release notes (fanquake) 699f3429c674ef7c42cc275dc093aa8f84e1bb34 doc: add historical 0.20.2 release notes (fanquake) Pull request description: 0.20.2 and 0.21.2 are missing from master. ACKs for top commit: jarolrod: ACK cb075d245e3a9106783f5edd0ed2cf1f7e43cdf8 Tree-SHA512: f05fb2e5b589cd60581e724182c6f32f992a85e6dc41f5a91d5c6941869ff4c0a7f28a405b3dfc71d9660c1385a1a13d220aa50b2d69c7787cb7974a3e4bf814
2022-10-04build, msvc: Enable C4834 warningHennadii Stepanov
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c4834
2022-10-04test: Remove unused fCheckpointsEnabled from miner_testsMacroFake
The earliest checkpoint is at height 11111, so this can't possibly have any impact on this test.