aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-09-12Merge bitcoin/bitcoin#30883: build: Revert "Minimize I/O operations in ↵merge-script
`GenerateHeaderFrom{Json,Raw}.cmake`" fdeb717e78fd55fbfda199c87002358bdc5895af Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`" (Hennadii Stepanov) Pull request description: This reverts commit b07fe666f27e2b2515d2cb5a0339512045e64761 from https://github.com/bitcoin/bitcoin/pull/30842. Fixes https://github.com/bitcoin/bitcoin/issues/30881. Apparently, the `string(APPEND ...)` command isn't optimized for large strings. ACKs for top commit: maflcko: review ACK fdeb717e78fd55fbfda199c87002358bdc5895af Tree-SHA512: ad5c3d49d3395ab318edcd7c9a98090838bec0cd3c1f1cc6ebc6f4262df2494f605458b523251bf5e590bbcfda15ed963f0a814678135ce4cc2dca9a108d20c7
2024-09-12Merge bitcoin/bitcoin#30814: kernel: Create usable static kernel librarymerge-script
0dd16d7118f10ac291a45644769121cbdfd2352f build: Add a pkg-config file for libbitcoinkernel (TheCharlatan) 45be32f8384398ad8d590137d05a6373aa827b75 build: Produce a usable static kernel library (TheCharlatan) Pull request description: Since the move to cmake, the kernel static library that is installed after a cmake --install build is unusable. It lacks symbols for the internal libraries, besides those defined in the kernel library target. Fix this by explicitly installing all the required internal static libraries. To make usage of these installed libraries easy, add a pkg-config file that can be used during linking. This patch can be tested with: ``` cmake -B build -DBUILD_SHARED_LIBS=OFF -DBUILD_KERNEL_LIB=ON cmake --build build cmake --install build g++ -std=c++20 -o test_chainstate src/bitcoin-chainstate.cpp -I/home/drgrid/bitcoin/src $(pkg-config --libs --static libbitcoinkernel) ``` Attempts to solve #30801 ACKs for top commit: hebasto: ACK 0dd16d7118f10ac291a45644769121cbdfd2352f. fanquake: ACK 0dd16d7118f10ac291a45644769121cbdfd2352f - this looks like a good place to start. ryanofsky: Code review ACK 0dd16d7118f10ac291a45644769121cbdfd2352f Tree-SHA512: 92f7bc959584bdc595f4aa6d0ab133355481075fe8564224fd7ac122fd7bdd75f98cf26ef0a6a7d84fd552d2258ddca1b674eca91122469a58bacc5f0a0ec2ef
2024-09-12Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"Hennadii Stepanov
This reverts commit b07fe666f27e2b2515d2cb5a0339512045e64761.
2024-09-12refactor: move m_is_inbound out of CNodeStateSergi Delgado Segura
`m_is_inbound` cannot be changed throughout the life of a `Peer`. However, we are currently storing it in `CNodeState`, which requires locking `cs_main` in order to access it. This can be moved to the outside scope and only require `m_peer_mutex`. This is a refactor in preparation for Erlay reworks.
2024-09-12test: Wait for local services to update in feature_assumeutxoFabian Jahr
2024-09-12Merge bitcoin/bitcoin#30872: test: fix exclude parsing for functional runnermerge-script
72b46f28bf8d37a166961b5aa2a22302b932b756 test: fix exclude parsing for functional runner (Max Edwards) Pull request description: This restores previous behaviour of being able to exclude a test by name without having to specify .py extension. It was noticed in https://github.com/bitcoin/bitcoin/issues/30851 that tests were no longer being excluded. PR https://github.com/bitcoin/bitcoin/pull/30244 introduced being able to exclude a specific tests based on args (such as `--exclude "rpc_bind.py --ipv6`) but it made the wrong assumption that test names intended to be excluded would include the .py extension. The following https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 shows that this is not how the `--exclude` flag was used in CI. https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 gave three examples of `--exclude` being used in CI so I compared the number of tests that the runner would run for these three examples in three situations, before #30244 was introduced, in master today and with this PR applied. Example: `--previous-releases --coverage --extended --exclude feature_dbcrash` Test count: Before #30244 introduced: 314 Master: 315 With this PR: 314 Example: `--exclude feature_init,rpc_bind,feature_bind_extra` Test count: Before #30244 introduced: 306 Master 311 With this PR: 306 Example: `--exclude rpc_bind,feature_bind_extra` Before #30244 introduced: 307 Master 311 With this PR: 307 I've also tested that the functionality introduced with #30244 remains and we can still exclude specific tests by argument. ACKs for top commit: maflcko: review ACK 72b46f28bf8d37a166961b5aa2a22302b932b756 willcl-ark: ACK 72b46f28bf8d37a166961b5aa2a22302b932b756 Tree-SHA512: 37c0e3115f4e3efdf9705f4ff8cd86a5cc906aacc1ab26b0f767f5fb6a953034332b29b0667073f8382a48a2fe9d649b7e60493daf04061260adaa421419d8c8
2024-09-12Merge bitcoin/bitcoin#30865: build: Skip secp256k1 ctime tests when tests ↵merge-script
are not being built 23eedc5d1e24b3d31da7902ece5182b9b24672d8 build: Skip secp256k1 ctime tests when tests are not being built (Hennadii Stepanov) Pull request description: Fixes https://github.com/bitcoin/bitcoin/pull/30791#issuecomment-2340860619: > Building with a fuzz engine fails, because the ctime tests are auto-detected in cmake, based on whether or not valgrind-devel is installed or not. ACKs for top commit: maflcko: re-review ACK 23eedc5d1e24b3d31da7902ece5182b9b24672d8 fanquake: ACK 23eedc5d1e24b3d31da7902ece5182b9b24672d8 Tree-SHA512: bfc0f2798acd36be9c52073d578b42c002606c60ef3fe8ef633eaea4f5382a3e9765d31637e4c25d8b71fd70473b29c24af4732e55e5183f27b48725b61fa15b
2024-09-12build: Skip secp256k1 ctime tests when tests are not being builtHennadii Stepanov
Co-authored-by: fanquake <fanquake@gmail.com>
2024-09-12util: Use compile-time check for LogConnectFailureMarcoFalke
2024-09-12util: Use compile-time check for FatalErrorfMarcoFalke
2024-09-12util: Add ConstevalFormatStringMarcoFalke
The type is used to wrap a format string once it has been compile-time checked to contain the right number of format specifiers.
2024-09-12test: fix exclude parsing for functional runnerMax Edwards
This restores previous behaviour of being able to exclude a test by name without having to specify .py extension.
2024-09-12Merge bitcoin/bitcoin#30733: test: remove unused src_dir param from ↵merge-script
run_tests after CMake migration 2ad560139b80e149844f285cd1456bb7cc0eb1ee Remove unused src_dir param from run_tests (Lőrinc) Pull request description: The `src_dir` usage was removed in https://github.com/bitcoin/bitcoin/commit/a8a2e364acf55bbe18404ab21f852d52257bcb6d#diff-437d7f6e9f2229879b60aae574a8217f14c643bbf3cfa9225d8011d6d52df00cL598, making the parameter unused. Top commit has no ACKs. Tree-SHA512: 1fd8b93811b4ab467ba5a160a4fe204e9606e1bf237c7595ed6f8b7821cf59d2a776c0e1e154852a45b2a35e5bdbd8996314e4f63a9c750f21b9a17875cb636a
2024-09-12Merge bitcoin/bitcoin#30871: build: Add more cmake presetsmerge-script
f15e817811e328423ea489870ead089128a6ef8c build: add more CMake presets (dev-mode, libfuzzer, libfuzzer-nosan) (Pieter Wuille) Pull request description: Add three more cmake presets to the project-wide `CMakePresets.json` file: * `dev-mode`: enables all features and dependencies * `libfuzzer`: builds for fuzzing with libfuzzer and the typical sanitizers (but not the optional ones that require suppressions) enabled. * `libfuzzer-nosan`: builds for fuzzing with libfuzzer and no (other) sanitizers ... and then uses these in some documentation. ACKs for top commit: ryanofsky: Code review ACK f15e817811e328423ea489870ead089128a6ef8c. This change is much needed to simplify my command line. TheCharlatan: ACK f15e817811e328423ea489870ead089128a6ef8c Tree-SHA512: a5f67bb7119fd36832ca5eb7189db04bfaf88f954aa461bfb2aeb866469057b0d0272835c418bc3a264c30dd8fba6d2e2cc8a6741a889f28f52c1c09b3ba9704
2024-09-12Merge bitcoin/bitcoin#27038: security-check: test for `_FORTIFY_SOURCE` ↵merge-script
usage in release binaries be4f78275fa6608b11377dd5a29a809597d3fe8d contrib: test for FORTIFY_SOURCE in security-check.py (fanquake) Pull request description: Test for the existence of fortified functions in the ELF release binaries. Currently skips `bitcoin-util` and checks for RISC-V. ACKs for top commit: TheCharlatan: ACK be4f78275fa6608b11377dd5a29a809597d3fe8d Tree-SHA512: decea5f359f1e673aa0119916f674f409a13b69db7da366cd95c1540201e117ff5a979da67bc2517fe786c2ac23d1006a9aaf662d7eadeec35da6aae4998c065
2024-09-12Merge bitcoin/bitcoin#30847: test: Drop no longer needed workaroundsmerge-script
5c80192ff6b982ee3a75be4142fe942b8206f2cd test: Drop no longer needed workarounds (Hennadii Stepanov) Pull request description: This PR deletes the workarounds introduced in https://github.com/bitcoin/bitcoin/pull/16564 and https://github.com/bitcoin/bitcoin/pull/15382, as `ctest` skips these cases gracefully: https://github.com/bitcoin/bitcoin/blob/5c80192ff6b982ee3a75be4142fe942b8206f2cd/src/test/CMakeLists.txt#L201-L203 ACKs for top commit: kevkevinpal: ACK [5c80192](https://github.com/bitcoin/bitcoin/pull/30847/commits/5c80192ff6b982ee3a75be4142fe942b8206f2cd) fanquake: ACK 5c80192ff6b982ee3a75be4142fe942b8206f2cd. Looks correct: Tree-SHA512: c47c606ecf7d64016b3c6353c3d4898350edc2caeac494dfd44484417f500a73f0c88c39f0f24651f3a02ef31ed9ca5c70d938bb9a8ca1eea54927e4d6a8fcd2
2024-09-12Merge bitcoin/bitcoin#30835: build: Introduce "Kernel" installation componentmerge-script
7b04fabe2d95f05a295b1ef30c9aeab7f753ed46 build: Introduce "Kernel" installation component (Hennadii Stepanov) Pull request description: This PR enables building and installing only `libbitcoinkernel`, without the need to disable other targets during the project build system generation: ``` $ rm -rf build && cmake -B build -DBUILD_KERNEL_LIB=ON $ cmake --build build --target bitcoinkernel $ cmake --install build --component Kernel --prefix /home/hebasto/INSTALL -- Install configuration: "RelWithDebInfo" -- Installing: /home/hebasto/INSTALL/lib/libbitcoinkernel.so ``` Please note, that only the `bitcoinkernel` target is being built. Related to https://github.com/bitcoin/bitcoin/issues/30801 and https://github.com/bitcoin/bitcoin/pull/30814. ACKs for top commit: TheCharlatan: ACK 7b04fabe2d95f05a295b1ef30c9aeab7f753ed46 ryanofsky: Code review ACK 7b04fabe2d95f05a295b1ef30c9aeab7f753ed46 Tree-SHA512: eac114dde059e47c91938a4a9108fc0fc693b5342ed3b6ecb971615be8ad3225b9985aae12d6ad18e673edf1bd39a5ecf259c1b61734f221669091bf2ce93a67
2024-09-12Merge bitcoin/bitcoin#30803: build: Minor build system fixes and amendmentsmerge-script
1cc93fe7b40f10a7d1d1189058af98a2bce31381 build: Delete dead code that implements `IF_CHECK_FAILED` option (Hennadii Stepanov) 341ad238091d4df520c70f1757b017e6f6620f24 build: Delete MSVC special case for `BUILD_FOR_FUZZING` option (Hennadii Stepanov) fdad128b528bc8622bc6d8343026c28b18260f64 build: Stop enabling CMake's CMP0141 policy (Hennadii Stepanov) b2a6f545b4f6e3442ae51f66a6f3c1de92d00a1b doc: Drop `ctest` command from Windows cross-compiling instructions (Hennadii Stepanov) 73b618582dcf06dd01be062fe0f81060cfcb48d8 build: Print `CMAKE_CXX_COMPILER_ARG1` in summary (Hennadii Stepanov) f03c9420958de31fdfecec5fa3e23134aac61803 build, test: Add missed log options (Hennadii Stepanov) 6f2cb0eafdef81fb9464a4679c3a5905d19e5103 doc: Amend comment about ZeroMQ config files (Hennadii Stepanov) Pull request description: This PR addresses the following comments: - https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742342524 - https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1728692369 - https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1736110362 - https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742931121 - https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1747723657 - https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742328675 - https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1723106474 ACKs for top commit: sipsorcery: tACK 1cc93fe7b40f10a7d1d1189058af98a2bce31381 (win11 msvc). maflcko: re-ACK 1cc93fe7b40f10a7d1d1189058af98a2bce31381 Tree-SHA512: a390797bb4d3b7eb9163653b6c9c324e7a01090f6cdda74df7349a24a5c4a2084e5912878747f56561315afc70cae9adb1c363f47ceb0af96004ea591d25171b
2024-09-12Merge bitcoin/bitcoin#30838: build: Use CMake's default permissions in macOS ↵merge-script
`deploy` target 5ba03e7d35e1b47ba864c9ae3c94af97cd3ae10b build: Use CMake's default permissions in macOS `deploy` target (Hennadii Stepanov) Pull request description: This PR ensures that the file permissions in macOS `zip` archives are independent of the user's `umask` value. Fixes https://github.com/bitcoin/bitcoin/issues/30815. ACKs for top commit: fanquake: ACK 5ba03e7d35e1b47ba864c9ae3c94af97cd3ae10b - I'm going to merge this now so we return to usable (comparable) guix builds. Tree-SHA512: 78f724cd3ffd5c1fd5fc1b4832f1e8154c62723f3de5ac9599f44715cbd08a3dfbb806801411c55069773d2e34c9f8cab25585dbad2f032c36b68dd83cb51847
2024-09-12Merge bitcoin/bitcoin#30841: ci: Post CMake-migration fixes and amendmentsmerge-script
c45186ca548362b75a6640393ccf79b11ff727da ci: Switch from `make` to `cmake --build` (Hennadii Stepanov) 6e5f33af581e0a74897a6cc9fad558b29ad8b88e ci: Handle log files regardless of CMake's version (Hennadii Stepanov) Pull request description: This PR addresses the change in logging that [happened](https://cmake.org/cmake/help/latest/release/3.26.html#configure-log) in CMake 3.26. Additionally, the `make` invocation replaced with `cmake --build`. Here are examples of the CI logs: - for a an error during the build system generation: https://cirrus-ci.com/build/5210987156996096 - for a compiler error: https://cirrus-ci.com/build/4617660913156096 ACKs for top commit: maflcko: review ACK c45186ca548362b75a6640393ccf79b11ff727da fanquake: ACK c45186ca548362b75a6640393ccf79b11ff727da Tree-SHA512: 2096f08c482ab9e10056cd4ec694ce40996243e2a1af2212dfff8cccbf0f51391d9a3dc396f7bba4f2877072a13a42bf667a02a44eab44e917aafb14d04e8e39
2024-09-12Merge bitcoin/bitcoin#30842: build: Minimize I/O operations in ↵merge-script
`GenerateHeaderFrom{Json,Raw}.cmake` b07fe666f27e2b2515d2cb5a0339512045e64761 build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake` (Hennadii Stepanov) Pull request description: This PR aims to reduce build time by replacing multiple `file(WRITE|APPEND ...)` commands with a single `file(WRITE ...)` command. Due to differences in implementation (e.g., filesystem design, system calls, caching), a noticeable improvement in build time is observed only on Windows. Additionally, the code has been refactored to remove the `remainder` local variables. ACKs for top commit: sipsorcery: tACK b07fe666f27e2b2515d2cb5a0339512045e64761 maflcko: review ACK b07fe666f27e2b2515d2cb5a0339512045e64761 TheCharlatan: ACK b07fe666f27e2b2515d2cb5a0339512045e64761 Tree-SHA512: 6ed3ae8fe7d8859af38d83918eddf7cb318607787863b95589f4a7a45a36f8c4bd1c01e366078d0515115c121bc857dc63471e52ff26fc49edbc8bb69875e947
2024-09-12Merge bitcoin/bitcoin#30867: build: Fix `ENABLE_WALLET` optionmerge-script
0037d53d1a21ec8a5a97a83ab716d68030446021 build: Fix `ENABLE_WALLET` option (Hennadii Stepanov) Pull request description: The removed commands were left over from the transition from autodetection to explicit options in the CMake staging branch. These commands prevented the `-DENABLE_WALLET=OFF` option from being work properly when building with depends. How to test: ``` $ make -C depends NO_QT=1 ``` On the master branch @ c66c68345efb0bb3d5613ebac703cde779fa0f01: ``` $ rm -rf build && cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake -DENABLE_WALLET=OFF < snip > Optional features: wallet support ...................... ON - descriptor wallets (SQLite) ...... ON - legacy wallets (Berkeley DB) ..... ON external signer ..................... ON < snip > ``` With this PR: ``` $ rm -rf build && cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake -DENABLE_WALLET=OFF < snip > Optional features: wallet support ...................... OFF external signer ..................... ON < snip > ACKs for top commit: maflcko: review ACK 0037d53d1a21ec8a5a97a83ab716d68030446021 kevkevinpal: ACK [0037d53](https://github.com/bitcoin/bitcoin/pull/30867/commits/0037d53d1a21ec8a5a97a83ab716d68030446021) pablomartin4btc: tACK 0037d53d1a21ec8a5a97a83ab716d68030446021 Tree-SHA512: 0eb14ef104f12a4205172d646c2af820e04514286b5b9a4ceb59c248ce880198dd4051d669098c46c0c0dce069bb60899d90509bbcae65cbeb958e52564fe920
2024-09-11Merge bitcoin/bitcoin#30807: Fix peers abruptly disconnecting from ↵Ava Chow
AssumeUTXO nodes during IBD 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac test: add coverage for assumeUTXO honest peers disconnection (furszy) 6d5812e5c852c233bd7ead2ceef051f8567619ed assumeUTXO: fix peers disconnection during sync (furszy) Pull request description: Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (`NODE_NETWORK`), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness from the AssumeUTXO node. This lack of response occurs because nodes ignore `getdata` requests when they do not have the block data available (further discussion can be found in #30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal `NODE_NETWORK_LIMITED` support. Then, full-node (`NODE_NETWORK`) support will be re-enabled once the background chain sync is completed. Thanks mzumsande for a post-#30385 convo too. Testing notes: Just cherry-pick the second commit (bb08c22) on master. It will fail there, due to the IBD node requesting historical blocks to the snapshot node - which is bad because the snapshot node will ignore the requests and stall + disconnect after some time. ACKs for top commit: achow101: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac naumenkogs: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac mzumsande: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac Tree-SHA512: fef525d1cf3200c2dd89a346be9c82d77f2e28ddaaea1f490a435e180d1a47a371cadea508349777d740ab56e94be536ad8f7d61cc81f6550c58b609b3779ed3
2024-09-11Merge bitcoin/bitcoin#30840: docs: Updated debug build instructions for cmakeAva Chow
0b003e1ff7e09b56cd013b15f1998495994b7211 docs: Updated debug build instructions for cmake (ion-) Pull request description: In the [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md) the section on [compiling for debug](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-debugging) displays outdated instructions that were applicable before the move to cmake build system. This PR just gives instructions on how to build for debugging in the context of cmake. ACKs for top commit: kevkevinpal: ACK [0b003e1](https://github.com/bitcoin/bitcoin/pull/30840/commits/0b003e1ff7e09b56cd013b15f1998495994b7211) achow101: ACK 0b003e1ff7e09b56cd013b15f1998495994b7211 tdb3: ACK 0b003e1ff7e09b56cd013b15f1998495994b7211 Tree-SHA512: fc8b4824d68e47b3e64320b4ce728df4242d00c2e8c75dcf5daa60af3050c62d94fa9741e447c7d81e72194470ed68a4e19bbee8cf6a1412d6566c7b699cf242
2024-09-11Merge bitcoin/bitcoin#30870: docs: updated developer notes for ↵Ava Chow
--with-sanitizers to -DSANITIZERS 4b1ce3cac81497dd094e8c34550edf633f4d38ae docs: updated developer notes for --with-sanitizers to -DSANITIZERS and removed resource for -fsanitze flags (kevkevinpal) Pull request description: In the developer notes we are incorrectly using the Autotools `--with-sanitizers` configure flag which we should now be using `cmake -B build -DSANITIZERS=<values>` instead now ACKs for top commit: maflcko: review ACK 4b1ce3cac81497dd094e8c34550edf633f4d38ae achow101: ACK 4b1ce3cac81497dd094e8c34550edf633f4d38ae pablomartin4btc: ACK 4b1ce3cac81497dd094e8c34550edf633f4d38ae Tree-SHA512: 029d55d802f6b4a85f3541f9a23e9ac85e6c590e91081204bfa737169138f61877883db51ad99cd8b802b0737eec35df5a33a5351b3e6b2f180f3ad0282d3616
2024-09-11build: add more CMake presets (dev-mode, libfuzzer, libfuzzer-nosan)Pieter Wuille
2024-09-11lint: Remove forbidden functions from lint-format-strings.pyMarcoFalke
Given that all of them are forbidden by the test/lint/lint-locale-dependence.py check, they can be removed.
2024-09-11docs: updated developer notes for --with-sanitizers to -DSANITIZERS and ↵kevkevinpal
removed resource for -fsanitze flags
2024-09-11Compare FromUserHex result against other hex validators and parsersLőrinc
2024-09-11Use BOOST_CHECK_EQUAL for optional, arith_uint256, uint256, uint160Lőrinc
Example error before: > unknown location:0: fatal error: in "validation_chainstatemanager_tests/chainstatemanager_args": std::bad_optional_access: bad_optional_access test/validation_chainstatemanager_tests.cpp:781: last checkpoint after: > test/validation_chainstatemanager_tests.cpp:801: error: in "validation_chainstatemanager_tests/chainstatemanager_args": check set_opts({"-assumevalid=0"}).assumed_valid_block == uint256::ZERO has failed [std::nullopt != 0000000000000000000000000000000000000000000000000000000000000000] Also added extra minimum_chainwork test to make it symmetric with assumevalid Co-authored-by: Ryan Ofsky <ryan@ofsky.org> Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
2024-09-10Merge bitcoin/bitcoin#30805: test: Add explicit onion bind to p2p_permissionsglozow
082779d6062c5b72f3497bb864e4dbb4373a3a4c test: Add explicit onion bind to p2p_permissions (Ava Chow) Pull request description: When the bind option is replaced in the bitcoin.conf, bitcoind will attempd to bind to the default tor listening port. If another bitcoind is running that is already bound to that port, the bind will fail which, since #22729, causes the test to fail. This failure can be avoided by explicitly binding the tor port when the bind is removed. ACKs for top commit: tdb3: ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c theStack: re-ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c glozow: ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c Tree-SHA512: 4acb69ea2e00aeacf9e7c9ab9595ceaf0e0d2adbd795602034b2184197d9bad54c7bc9f3da43ef9c52a71869fe96ba8c87fc5b7c37880f258f5a2aaab2b4046c
2024-09-10build: Fix `ENABLE_WALLET` optionHennadii Stepanov
The removed commands were left over from the transition from autodetection to explicit options. These commands prevented the `-DENABLE_WALLET=OFF` option from being work properly when building with depends.
2024-09-10test: add coverage for assumeUTXO honest peers disconnectionfurszy
Exercising and verifying the following points: 1. An IBD node can sync headers from an AssumeUTXO node at any time. 2. IBD nodes do not request historical blocks from AssumeUTXO nodes while they are syncing the background-chain. 3. The assumeUTXO node dynamically adjusts the network services it offers according to its state. 4. IBD nodes can fully sync from AssumeUTXO nodes after they finish the background-chain sync.
2024-09-10assumeUTXO: fix peers disconnection during syncfurszy
Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (NODE_NETWORK), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness (lack of response) from the AssumeUTXO node. This lack of response occurs because nodes ignore getdata requests when they do not have the block data available (further discussion can be found in PR 30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal 'NODE_NETWORK_LIMITED' support. Then, full-node ('NODE_NETWORK') support will be re-enabled once the background chain sync is completed.
2024-09-10test: Add explicit onion bind to p2p_permissionsAva Chow
When the bind option is replaced in the bitcoin.conf, bitcoind will attempd to bind to the default tor listening port. If another bitcoind is running that is already bound to that port, the bind will fail which, since #22729, causes the test to fail. This failure can be avoided by explicitly binding the tor port when the bind is removed.
2024-09-10Merge bitcoin/bitcoin#30773: Remove unsafe uint256S() and test-only uint160S()Ryan Ofsky
43cd83b0c7ba436b8ffc83d8a65e935714dffe70 test: move uint256_tests/operator_with_self to arith_uint256_tests (stickies-v) c6c994cb2b9af58b1c0e554255d1a3be785e8d56 test: remove test-only uint160S (stickies-v) 62cc4656e2bb38f80485a13d75b42add09a6b731 test: remove test-only uint256S (stickies-v) adc00ad728dd54084671398f8fa5c12be92d2bab test: remove test-only arith_uint256S (stickies-v) f51b237723b87db706cbce2939d20eb193332799 refactor: rpc: use uint256::FromHex for ParseHashV (stickies-v) Pull request description: _Continuation of #30569._ Since https://github.com/bitcoin/bitcoin/commit/fad2991ba073de0bd1f12e42bf0fbaca4a265508, `uint256S()` has been [deprecated](https://github.com/bitcoin/bitcoin/pull/30482/commits/fad2991ba073de0bd1f12e42bf0fbaca4a265508#diff-800776e2dda39116e889839f69409571a5d397de048a141da7e4003bc099e3e2R138) because it is less robust than the `base_blob::FromHex()` introduced in https://github.com/bitcoin/bitcoin/pull/30482. Specifically, it tries to recover from length-mismatches, recover from untrimmed whitespace, 0x-prefix and garbage at the end, instead of simply requiring exactly 64 hex-only characters. (see also https://github.com/bitcoin/bitcoin/pull/30532) This PR removes `uint256S()` (and `uint160S()`) completely, with no non-test behaviour change. Specifically, the main changes in this PR are: - the (minimal) last non-test usage of `uint256S()` in `ParseHashV()` is removed without behaviour change, which can partially be verified by cherry-picking and/or modifying [this test commit](https://github.com/stickies-v/bitcoin/commit/1f2b0fa86db2ed65476b68417aa1bf4c26026a19)). - the test usage of `uint{160,256}S()` is removed, largely replacing it with `uint{160,256}::FromHex()` where applicable, potentially modifying the test by removing non-hex characters or dropping the test entirely if removing non-hex characters makes it redundant - the now unused `uint{160,256}S()` functions are removed completely. - unit test coverage on converting `uint256` <-> `arith_uint256` through `UintToArith256()` and `ArithToUint256()` is beefed up, and `arith_uint256` tests are moved to `arith_uint256_tests.cpp`, removing the `uint256_tests.cpp` dependency on `uint256h`, mirroring how the code is structured. _Note: `uint256::FromUserHex()` exists to more leniently construct uint256 from user input, allowing "0x" prefixes and too-short-input, as safer alternative to `uint256S()` where necessary._ ACKs for top commit: l0rinc: reACK 43cd83b0c7ba436b8ffc83d8a65e935714dffe70 hodlinator: re-ACK 43cd83b0c7ba436b8ffc83d8a65e935714dffe70 ryanofsky: Code review ACK 43cd83b0c7ba436b8ffc83d8a65e935714dffe70. Only code change is a small refactoring which looks good. The rest of the PR is all test changes, which I only lightly reviewed, but seem to be positive and do what's described Tree-SHA512: 48147a4c6af671597df0f72c1b477ae4631cd2cae4645ec54d0e327611ff302c9899e344518c81242cdde82930f6ad23a3a7e6e0b80671816e9f457b9de90a5c
2024-09-10Merge bitcoin/bitcoin#30065: init: fixes file descriptor accountingRyan Ofsky
d4c7c4009da14c84576d43ab4a1241967cfa5ffc init: error out if -maxconnections is negative (Sergi Delgado Segura) c7736494813ad11e4ad53789104e70a19819f412 init: improves file descriptors accounting and docs (Sergi Delgado Segura) 29008a7ff43cf712a8438663e79cebdf698efae9 init: fixes fd accounting regarding poll/select (Sergi Delgado Segura) Pull request description: The current logic for file descriptor accounting is pretty convoluted and hard to follow. This is partially caused by the lack of documentation plus non-intuitive variable naming (which was more intuitive when fewer things were accounted for, but hasn't aged well). This has led to this accounting being error-prone and hard to maintain (as shown in the first commit of this PR). Redefine some of the constants, plus document what are we accounting for so this can be extended more easily Fixes #18911 ACKs for top commit: sr-gi: > ACK [d4c7c40](https://github.com/bitcoin/bitcoin/commit/d4c7c4009da14c84576d43ab4a1241967cfa5ffc) naumenkogs: ACK d4c7c4009da14c84576d43ab4a1241967cfa5ffc vasild: ACK d4c7c4009da14c84576d43ab4a1241967cfa5ffc TheCharlatan: ACK d4c7c4009da14c84576d43ab4a1241967cfa5ffc Tree-SHA512: 1524d10c8ad8f354f6ab9c244699adbcdae2dd7aba37de5b8f9e177c629e8a2cce0f6e8117e076dde3a592f5283bd30a4201db96a3c011e335c02d1fde7414bc
2024-09-10net: call `Select` with reachable networks in `ThreadOpenConnections`brunoerg
Calling `Select` with reachable networks can avoid unecessary calls and avoid exceed the max number of tries.
2024-09-10addrman: change `Select` to support multiple networksbrunoerg
2024-09-10build: Use CMake's default permissions in macOS `deploy` targetHennadii Stepanov
This change fixes reproducibility issue with macOS Guix builds.
2024-09-10net: add `All()` in `ReachableNets`brunoerg
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2024-09-10fuzz: Add harness for p2p headers syncmarcofleon
2024-09-10Merge bitcoin/bitcoin#30791: build: Use correct variable namemerge-script
2d68c3b1c2e4f8fb881efc3569506d426ee5155d build: Use correct variables when passing `-fsanitize` to libsecp256k1 (Hennadii Stepanov) Pull request description: This was overlooked after https://github.com/bitcoin-core/secp256k1/pull/1546. Also see: - https://github.com/bitcoin-core/secp256k1/pull/1600 - https://github.com/bitcoin/bitcoin/pull/30845 - https://github.com/hebasto/oss-fuzz/pull/9 ACKs for top commit: fanquake: ACK 2d68c3b1c2e4f8fb881efc3569506d426ee5155d Tree-SHA512: 1a149e2072fd471c3af2f8591ccd69bddc8060eb04246c7f5596d179608fb097293c4c7b17f237fcf9014d8fc1ddc727497554fa9535777243ac989672ab1a75
2024-09-09Merge bitcoin/bitcoin#30509: multiprocess: Add -ipcbind option to bitcoin-nodeAva Chow
30073e6b3a24cbe417c45cd5df6a3a2de0251e9d multiprocess: Add -ipcbind option to bitcoin-node (Russell Yanofsky) 73fe7d723084653671f2178ea1177a8627edfafa multiprocess: Add unit tests for connect, serve, and listen functions (Ryan Ofsky) 955d4077aac621697246bcb20a854ba97e37c519 multiprocess: Add IPC connectAddress and listenAddress methods (Russell Yanofsky) 4da20434d4d68b7933e39aca36faa6fd2264cc45 depends: Update libmultiprocess library for CustomMessage function and ThreadContext bugfix (Ryan Ofsky) Pull request description: Add `-ipcbind` option to `bitcoin-node` to make it listen on a unix socket and accept connections from other processes. The default socket path is `<datadir>/node.sock`, but this can be customized. This option lets potential wallet, gui, index, and mining processes connect to the node and control it. See examples in #19460, #19461, and #30437. Motivation for this PR, in combination with #30510, is be able to release a bitcoin core node binary that can generate block templates for a separate Stratum v2 mining service, like the one being implemented in https://github.com/Sjors/bitcoin/pull/48, that connects over IPC. Other things to know about this PR: - While the `-ipcbind` option lets other processes to connect to the `bitcoin-node` process, the only thing they can actually do after connecting is call methods on the [`Init`](https://github.com/bitcoin/bitcoin/blob/master/src/ipc/capnp/init.capnp#L17-L20) interface which is currently very limited and doesn't do much. But PRs [#30510](https://github.com/bitcoin/bitcoin/pull/30510), [#29409](https://github.com/bitcoin/bitcoin/pull/29409), and [#10102](https://github.com/bitcoin/bitcoin/pull/10102) expand the `Init` interface to expose mining, wallet, and gui functionality respectively. - This PR is not needed for [#10102](https://github.com/bitcoin/bitcoin/pull/10102), which runs GUI, node, and wallet code in different processes, because [#10102](https://github.com/bitcoin/bitcoin/pull/10102) does not use unix sockets or allow outside processes to connect to existing processes. [#10102](https://github.com/bitcoin/bitcoin/pull/10102) lets parent and child processes communicate over internal socketpairs, not externally accessible sockets. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722). ACKs for top commit: achow101: ACK 30073e6b3a24cbe417c45cd5df6a3a2de0251e9d TheCharlatan: Re-ACK 30073e6b3a24cbe417c45cd5df6a3a2de0251e9d itornaza: Code review ACK 30073e6b3a24cbe417c45cd5df6a3a2de0251e9d Tree-SHA512: 2b766e60535f57352e8afda9c3748a32acb5a57b2827371b48ba865fa9aa1df00f340732654f2e300c6823dbc6f3e14377fca87e4e959e613fe85a6d2312d9c8
2024-09-09Add std::optional support to Boost's equality checkLőrinc
Also moved the operators to the bottom of the file since they're less important and to group them together. Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> Co-authored-by: stickies-v <stickies-v@protonmail.com>
2024-09-09Merge bitcoin/bitcoin#30817: test: Add coverage for dumptxoutset failure ↵Ava Chow
robustness c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 refactor: Manage dumptxoutset RAII classes with std::optional (Fabian Jahr) 4b5bf335adabd1586043caa72a98356a8255bc29 test: Add coverage for failing dumptxoutset behavior (Fabian Jahr) Pull request description: This adds a test that checks that network activity is not suspended if dumptxoutset fails in the middle of its process which is implemented with the `NetworkDisable` RAII class. I would have liked to add coverage for the `TemporaryRollback` RAII class but that seems a lot more tricky since the failure needs to happen at some point after the rollback and on the scale of our test chain here I couldn't find a way to do it yet. This was requested by pablomartin4btc here: https://github.com/bitcoin/bitcoin/pull/30808#pullrequestreview-2280450117. To test the test you can comment out the content of the destructor of `NetworkDisable`. It also addresses the feedback by ryanofsky to use `std::optional` instead of `std::unique_ptr` for the management of the RAII object: https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1744149228 ACKs for top commit: achow101: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 pablomartin4btc: cr & tACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 tdb3: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 BrandonOdiwuor: Code Review ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 theStack: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 Tree-SHA512: 9556e75014a2599bb870b70faf887608b332f2312626333f771d4ec11c04f863a2cf17e223ec473d4e8b0c9e8008394a4e0c321561f7ef3a2eec713dcfaea58a
2024-09-09Merge bitcoin/bitcoin#30684: init: fix init fatal error on invalid negated ↵Ava Chow
option value ee47ca29d6ef55650a0af63bca817c5d494f31ef init: fix fatal error on '-wallet' negated option value (furszy) Pull request description: Currently, if users provide a double negated value such as '-nowallet=0' or a non-boolean convertible value to a negated option such as '-nowallet=not_a_boolean', the initialization process results in a fatal error, causing an unclean shutdown and displaying a poorly descriptive error message: "JSON value of type bool is not of expected type string." (On bitcoind. The GUI does not display any error msg - upcoming PR -). This PR fixes the issue by ensuring that only string values are returned in the the "wallet" settings list, failing otherwise. It also improves the clarity of the returned error message. Note: This bug was introduced in https://github.com/bitcoin/bitcoin/pull/22217. Where the `GetArgs("-wallet")` call was replaced by `GetSettingsList("-wallet")`. ACKs for top commit: achow101: ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef ryanofsky: Code review ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef, just adding the suggested test since last review TheCharlatan: ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef ismaelsadeeq: Tested ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef Tree-SHA512: 5f01076f74a048019bb70791160f0accc2db7a457d969cb23687bed81ccbbdec1dda68311e7c6e2dd56250e23e8d926d4066e5014b2a99a2fc202e24ed264fbd
2024-09-09Merge bitcoin/bitcoin#30401: fix: increase consistency of rpcauth parsingAva Chow
27c976d11a68d66db97d9a7a30c6a6a71c6ab586 fix: increase consistency of rpcauth parsing (tdb3) 2ad3689512a36eaff957df9ac28e65b2fedbc36c test: add norpcauth test (tdb3) 67df0dec1abe547773e532aa60aff0317e018123 test: blank rpcauth CLI interaction (tdb3) ecc98ccff25b7e758337e764e59d764076772fec test: add cases for blank rpcauth (tdb3) Pull request description: The current `rpcauth` parsing behavior is inconsistent and unintuitive (see https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-1972085251 and additional details below). The current behavior inconsistently treats empty `rpcauth` as an error (or not) depending on the location within CLI/bitcoin.conf and the location of adjacent valid `rpcauth` params. Empty `rpcauth` is now consistently treated as an error and prevents bitcoind from starting. Continuation of the upforgrabs PR #29141. ### Additional details: Current `rpcauth` behavior is nonsensical: - If an empty `rpcauth` argument was specified as the last command line argument, it would cause all other `rpcauth` arguments to be ignored. - If an empty `rpcauth` argument was specified on the command line followed by any nonempty `rpcauth` argument, it would cause an error. - If an empty `rpcauth=` line was specified after non-empty rpcauth line in the config file it would cause an error. - If an empty `rpcauth=` line in a config file was first it would cause other rpcauth entries in the config file to be ignored, unless there were `-rpcauth` command line arguments and the last one was nonempty, in which case it would cause an error. New behavior is simple: - If an empty rpcauth config line or command line argument is used it will cause an error ACKs for top commit: naiyoma: Tested ACK [https://github.com/bitcoin/bitcoin/commit/27c976d11a68d66db97d9a7a30c6a6a71c6ab586](https://github.com/bitcoin/bitcoin/commit/27c976d11a68d66db97d9a7a30c6a6a71c6ab586) achow101: ACK 27c976d11a68d66db97d9a7a30c6a6a71c6ab586 ryanofsky: Code review ACK 27c976d11a68d66db97d9a7a30c6a6a71c6ab586. Since last review commit message was just tweaked to clarify previous behavior. Tree-SHA512: af2e9dd60d1ad030409ae2c3805ab139c7435327823d9f8bbeede815f376cb696a5929b08a6e8c8b5f7278ed49cfb231789f9041bd57f1f03ec96501b669da5b
2024-09-09build: Use correct variables when passing `-fsanitize` to libsecp256k1Hennadii Stepanov
This was overlooked after https://github.com/bitcoin-core/secp256k1/pull/1546
2024-09-09Merge bitcoin/bitcoin#30845: Update libsecp256k1 subtree to latest mastermerge-script
611562806cf3fd3028e24e6c5a8e8dcb8805be38 Squashed 'src/secp256k1/' changes from 642c885b61..2f2ccc4695 (Hennadii Stepanov) Pull request description: This PR updates the libsecp256k1 subtree to https://github.com/bitcoin-core/secp256k1/commit/2f2ccc469540fde6495959cec061e95aab033148, which includes the following changes: - https://github.com/bitcoin-core/secp256k1/pull/1577 - https://github.com/bitcoin-core/secp256k1/pull/1578 - https://github.com/bitcoin-core/secp256k1/pull/1583 - https://github.com/bitcoin-core/secp256k1/pull/1586 - https://github.com/bitcoin-core/secp256k1/pull/1600 The latter is required for https://github.com/bitcoin/bitcoin/pull/30791. ACKs for top commit: l0rinc: utACK ff54395de421fa1cbcf80ddbca4d71f0d2a4d12f real-or-random: utACK https://github.com/bitcoin/bitcoin/pull/30845/commits/ff54395de421fa1cbcf80ddbca4d71f0d2a4d12f no blockers from libsecp256k1 side, and these commits touch only build/docs/tests and are thus particularly harmless fanquake: ACK ff54395de421fa1cbcf80ddbca4d71f0d2a4d12f Tree-SHA512: 77cf1ba9aa24efdcf01e99850ea8bed54f847307a3c98c10289c9b7fd9e70c9219f28bee0f00ef7d69979d95a0ddac1e937d3d183ebc9d9b8e6cce0db1d507c9
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-17 23:53:02 +0000