Iwyu compat - #341
Open
kevkevinpal wants to merge 490 commits into
Open
Conversation
779e782 fuzz: wallet: add target for `MigrateToDescriptor` (brunoerg) Pull request description: This PR adds fuzz coverage for the scriptpubkeyman migration (`MigrateToDescriptor`). Note that it's a test for the migration of the scriptpubkey manager, not for the whole migration process as tried in bitcoin#29694, because: 1) The wallet migration deals with DBs which is expensive for fuzzing (was getting around 3 exec/s); 2) Mocking would require lots of refactors. This target loads keys, HDChain (even inactive ones), watch only and might add tons of different scripts, then calls `MigrateToDescriptor`. It does not play with encrypted stuff because it would make the target super slow. Also, after the migration there are some assertions that would work as a regression test for bitcoin#31452, for example. ACKs for top commit: frankomosh: Code Review ACK 779e782 marcofleon: reACK 779e782 Tree-SHA512: 08ef5166602c21658765bc063c5421e81055d094d346c4e2a28215209c6b7768b99a424f3ba47cf718dc8d827a588da22394ba23402a40a71a976d80d65e6c2e
… signals d6f680b validation: Move block into BlockDisconnected signal (sedited) 4d02d2b validation: Move block into BlockConnected signal (sedited) 8b0fb64 validation: Move validation signal events to task runner (sedited) Pull request description: This enforces behaviour that is currently already implicit: The destructor for blocks runs mostly in the [scheduler thread](https://bitcoin-dev-tools.github.io/benchcoin/results/pr-176/20472174834/mainnet-default-instrumented-head-flamegraph.svg?x=2762391536960&y=684). The change should make it a bit clearer what the ownership semantics for these validation signals are. `BlockConnected` already takes a reference to a block that is emplaced in `connected_blocks`. Once `connected_blocks` is iterated through, it is not reused. Similarly `BlockDisconnected` currently takes a reference to a block that is discarded after the call to it. Note that this does not give the guarantee that blocks' lifetimes are extended by other means once they are connected. For example after IBD, the block's lifetime is extended in net_processing's `m_most_recent_block` and `ActivateBestChain` itself takes a copy of the block's shared pointer, meaning its caller may delay de-allocation. ACKs for top commit: maflcko: re-review ACK d6f680b 🔌 stickies-v: re-ACK d6f680b frankomosh: Re-ACK d6f680b Tree-SHA512: 9209a7d23e7af0d76fa70dff958b1329f38ef29ccc49b5a32bcf9f349d59cc2bf70464ebdb130d26077c0ff9362ce9211472231d375ff1c9c89c0ec3020eac80
Replace the elided "JSONRPC error" placeholder in the importdescriptors RPC help with explicit "code" and "message" fields. This makes the documented result schema match the structured error object returned in practice.
On Windows, the `winerror.h` header defines `WAIT_TIMEOUT` as a macro. This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before using `WAIT_TIMEOUT`, the preprocessor expands it into a numeric literal, causing syntax errors. Rename the variable to `TEST_WAIT_TIMEOUT` to remove this fragility and avoid the collision entirely. -BEGIN VERIFY SCRIPT- sed -i 's/\<WAIT_TIMEOUT\>/TEST_WAIT_TIMEOUT/g' $(git grep -l 'WAIT_TIMEOUT' ./src/) -END VERIFY SCRIPT-
…elds (`nLockTime`, `nSequence`) 9f28120 kernel: Add API function for getting a tx input's nSequence (Sebastian Falbesoner) 6b64b18 kernel: Add API function for getting a tx's nLockTime (Sebastian Falbesoner) Pull request description: This PR introduces two new C API functions to libbitcoinkernel: * `btck_transaction_get_locktime` to access a transaction's `nLockTime` value * `btck_transaction_input_get_sequence` to access a transaction input's `nSequence` value Inspired by https://bnoc.xyz/t/forward-compatible-coinbase-locktimes-for-bip-54. After reading this I thought it would be a nice/useful showcase to check BIP54 compliance of (historical) blocks using bitcoinkernel, without having to manually deserialize the transaction (this is just about one of the four BIP54 rules though, especially the sigops limit is much more involved). ACKs for top commit: sedited: ACK 9f28120 yuvicc: ACK 9f28120 stickies-v: ACK 9f28120 Tree-SHA512: 9eae795d6e4b9b367bbfe2665b916121ef64031e8d10667c71741344b5eea4c2562862a937bdf1363cc66b67bb5d48392c9f44e52f0d92d2a5a65e10d061b703
…in#31713 follow-ups) 39e3295 test(miniscript): Check for depth rather than script size (Hodlinator) 5af5e87 test(miniscript): Make tested script valid (Hodlinator) fd7c494 doc(miniscript): Explain why we operate on vectors (Hodlinator) da51b5e refactor(miniscript): Move keys to avoid copy (Hodlinator) Pull request description: * Add missing move bitcoin#31713 (comment) * Document reason behind moving entire `vector`s bitcoin#31713 (comment) * Make miniscript valid and improve test name bitcoin#31713 (comment) * Check for miniscript node depth rather than script size bitcoin#31713 (comment) ACKs for top commit: l0rinc: ACK 39e3295 sedited: ACK 39e3295 darosior: Github light ACK 39e3295. Code looks correct to me. I don't understand why i'm co-author of the last commit, since i did not author any code in there. 🤷 Tree-SHA512: 88c240183d7ebe93e3a1d3d65969b435775190f15d2845b58dbd16938553bb6490ab57400544f90a5a3f9a73245dce769ffc4868ae6fb7513f7db46743bfb9e1
When a large cluster of unconfirmed transactions exceeds the limit, calculateCombinedBumpFee() returns std::nullopt. Previously, we continued executing and the optional value was accessed unconditionally, leading to a std::bad_optional_access exception. Fix this by returning early when the returned bumped fee is null. Note: This is a crash for the GUI, and an uncaught exception for the RPC bumpfee and psbtbumpfee.
Including the missing `<span>` header in `serialize.h` allows IWYU to correctly evaluate its redundancy elsewhere.
…s, Enable -Wcovered-switch-default fa4ec13 build: Enable -Wcovered-switch-default (MarcoFalke) fa2670b refactor: Enable -Wswitch in exhaustive switch (MarcoFalke) Pull request description: The compiler flag `-Wswitch` is enabled. However, it can not fire when a `default:` case exists. Fix that by removing the default case where a switch is already handling all cases exhaustively. Also, enable `-Wcovered-switch-default` to catch those cases at compile time in the future. Also, apply the comment according to the dev notes. Can be reviewed via `--ignore-all-space` ACKs for top commit: stickies-v: re-ACK fa4ec13, no changes except for addressing silent merge conflict from d339884 l0rinc: ACK fa4ec13 achow101: ACK fa4ec13 sedited: ACK fa4ec13 Tree-SHA512: 8dd9e71a8cd338255f43448a59a1a4d40a9fc16e19a707cc10fb71442d4df9f82a0e5fae77868ef49cd0ea27fdd972687572c1a50b6aba7e08c6ce87576afc6a
…standard d8f4e7c doc: add release notes (ismaelsadeeq) 248c175 test: ensure `ValidateInputsStandardness` optionally returns debug string (ismaelsadeeq) d2716e9 policy: update `AreInputsStandard` to return error string (ismaelsadeeq) Pull request description: This PR is another attempt at bitcoin#13525. Transactions that fail `PreChecks` Validation due to non-standard inputs now returns invalid validation state`TxValidationResult::TX_INPUTS_NOT_STANDARD` along with a debug error message. Previously, the debug error message for non-standard inputs do not specify why the inputs were considered non-standard. Instead, the same error string, `bad-txns-nonstandard-inputs`, used for all types of non-standard input scriptSigs. This PR updates the `AreInputsStandard` to include the reason why inputs are non-standard in the debug message. This improves the `Precheck` debug message to be more descriptive. Furthermore, I have addressed all remaining comments from bitcoin#13525 in this PR. ACKs for top commit: instagibbs: ACK bitcoin@d8f4e7c achow101: ACK d8f4e7c sedited: Re-ACK d8f4e7c Tree-SHA512: 19b1a73c68584522f863b9ee2c8d3a735348667f3628dc51e36be3ba59158509509fcc1ffc5683555112c09c8b14da3ad140bb879eac629b6f60b8313cfd8b91
…1c97fde2809c3451624d3 3293e9a guix: document when GCC SSA gen patch can be removed (fanquake) 978023f guix: use latest glibc 2.31 (fanquake) ab9a98b guix: combine gcc-libgcc-patches with base-gcc (fanquake) 2276426 guix: switch to upstream python-oscrypto package (fanquake) feea2a8 ci: use LIEF 0.17.5 in lint job (fanquake) a7524f5 guix: switch to upstream python-lief package (fanquake) 2bf97e8 guix: switch to upstream osslsigncode package (fanquake) dc0ddab guix: drop CMake workaround (fanquake) 31eb46f guix: update to c5eee3336cc1d10a3cc1c97fde2809c3451624d3 (fanquake) 0f323e1 guix: add --no-same-owner to TAR_OPTIONS (fanquake) Pull request description: Update the Guix time-machine to [c5eee3336cc1d10a3cc1c97fde2809c3451624d3](https://codeberg.org/guix/guix/commit/c5eee3336cc1d10a3cc1c97fde2809c3451624d3). Package updates: binutils 2.41 -> 2.44 cmake-minimal 3.24.2 -> 3.31.10 diffutils 3.10 -> 3.12 file 5.45 -> 5.46 git-minimal 2.46.0 -> 2.52.0 (base) glibc 2.39 -> 2.41 gzip 1.13 -> 1.14 linux-headers 6.1.119 -> 6.1.166 LLVM/Clang 19.1.4 -> 19.1.7 mingw-w64 12.0.0 -> 13.0.0 nsis 3.10 -> 3.11 python-minimal 3.10 -> 3.11 CMake 4.x becomes available. Clang/LLVM 20 & 21 become available. Switch to upstream `osslsigncode` (2.13, `python-lief` (0.17.5) and `python-oscrypto` (1.3.0) packages. Update glibc to the latest commit on the `2.31` branch. Could be used for bitcoin#32764. ACKs for top commit: pinheadmz: ACK 3293e9a achow101: ACK 3293e9a sedited: ACK 3293e9a willcl-ark: ACK 3293e9a Tree-SHA512: 46fe11281911be44ef34d9f19b4c2678f2e4628b4badc12ea4cc17cf3e1d693060cc70f0030b0ac53efae1172ceb61af447368dc4d69f5b2707c06e99206d6cf
…stalling. e76e886 guix: doc: zdiff3 doesn't work (David Gumberg) ea1be38 guix: doc: Suggest guix-install.sh --uninstall (David Gumberg) Pull request description: Add a note about guix issues with git `merge.conflictstyle` being set to `zdiff3`, and add information about how to uninstall for users of `guix-install.sh` ACKs for top commit: achow101: ACK e76e886 hebasto: ACK e76e886. sedited: utACK e76e886 Tree-SHA512: e9edc49851579c52e7ac762221d0bb51c928da4dff5566ece766af7f7eb75466831a76f885ccc5a26207c84b846e3f42bae26e80ddece3df1611de2c2320c28a
…ine option ddf2a06 Fix compatibility with `-debuglogfile` command-line option (Hennadii Stepanov) Pull request description: This PR avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option. ACKs for top commit: maflcko: review ACK ddf2a06 🐪 achow101: ACK ddf2a06 sedited: ACK ddf2a06 Tree-SHA512: bcf8f4e9407ed6108dbb7bfee673a6a486cd0afaaf1dcfb7aaa2d2c6a85488f9c57557b53033efa5eed723ec842c87c4616b6c078f049787e634cc02cbb740ea
…oad failures fa71c6e ci: Avoid intermittent Windows generate download failures (MarcoFalke) Pull request description: The CI Windows step to generate the build system is problematic, because with a clean cache, it will download artifacts over the network. This may intermittently fail due to intermittent network issues. Fix this issue, like all other network issues in CI, by retrying once. ACKs for top commit: hodlinator: ACK fa71c6e hebasto: ACK fa71c6e, I have reviewed the code and it looks OK. Tree-SHA512: 9915e84b6015116cce86a25b4f77e545baef1f26fabe767c8bb07a009b075f76c27f9172dd23f78aa8017fd611cea850b1e985a357bd0b24543e63e1fd85cb6d
…IT_TIMEOUT` 658e68f scripted-diff: Rename `WAIT_TIMEOUT` to `TEST_WAIT_TIMEOUT` (Hennadii Stepanov) Pull request description: On Windows, the `winerror.h` header defines `WAIT_TIMEOUT` as a macro. This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before using `WAIT_TIMEOUT`, the preprocessor expands it into a numeric literal, causing syntax errors. Rename the variable to `TEST_WAIT_TIMEOUT` to remove this fragility and avoid the collision entirely. Split from bitcoin#34448. Similar to bitcoin#34454. ACKs for top commit: w0xlt: ACK 658e68f Tree-SHA512: 90cf8927e4e41dee24d51fb2ea3335526ff5da4180c24d776d59b642794715b3c6558628088fbc28236d6ac4fffb9a27167fe309cb94ebf04f04c6e5cf957ad5
b14f2c7 tests: applied PYTHON_GIL to the env for every test (kevkevinpal) Pull request description: ## Summay If a user is running python3.14.0t they would see a warning log that would fail the integration test suite. This change adds `PYTHON_GIL=1` to the env when running our functional test suite to ensure that the tests pass for users running python3.14.0t and are not manually setting `PYTHON_GIL=1`. This resolves bitcoin#33582 ### Tests before and after #### Before ``` ./build/test/functional/test_runner.py interface_ipc.py Temporary test directory at /tmp/test_runner_₿_🏃_20260319_142327 Remaining jobs: [interface_ipc.py] 1/1 - interface_ipc.py failed, Duration: 2 s stdout: 2026-03-19T18:23:27.330123Z TestFramework (INFO): PRNG seed is: 4933091336597497631 2026-03-19T18:23:27.380917Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20260319_142327/interface_ipc_0 2026-03-19T18:23:28.625944Z TestFramework (INFO): Running echo test 2026-03-19T18:23:28.635856Z TestFramework (INFO): Running mining test 2026-03-19T18:23:28.648965Z TestFramework (INFO): Running deprecated mining interface test 2026-03-19T18:23:28.653589Z TestFramework (INFO): Running disconnect during BlockTemplate.waitNext 2026-03-19T18:23:28.821124Z TestFramework (INFO): Running thread busy test 2026-03-19T18:23:29.195589Z TestFramework (INFO): Stopping nodes 2026-03-19T18:23:29.299135Z TestFramework (INFO): Cleaning up /tmp/test_runner_₿_🏃_20260319_142327/interface_ipc_0 on exit 2026-03-19T18:23:29.299329Z TestFramework (INFO): Tests successful stderr: <frozen importlib._bootstrap>:491: RuntimeWarning: The global interpreter lock (GIL) has been enabled to load module 'capnp.lib.capnp', which has not declared that it can run safely without the GIL. To override this behavior and keep the GIL disabled (at your own risk), run with PYTHON_GIL=0 or -Xgil=0. TEST | STATUS | DURATION interface_ipc.py | ✖ Failed | 2 s ALL | ✖ Failed | 2 s (accumulated) Runtime: 2 s ``` #### After ``` ./build/test/functional/test_runner.py interface_ipc.py Temporary test directory at /tmp/test_runner_₿_🏃_20260319_142221 Remaining jobs: [interface_ipc.py] 1/1 - interface_ipc.py passed, Duration: 2 s TEST | STATUS | DURATION interface_ipc.py | ✓ Passed | 2 s ALL | ✓ Passed | 2 s (accumulated) Runtime: 2 s ``` ACKs for top commit: maflcko: review ACK b14f2c7 fanquake: ACK b14f2c7 Tree-SHA512: e5862d2e9211154d4834c88864e8c4e35de195986511ba151871d39266d177e0718960b28020e815ef6b353a0d82800b7cb68e9a6dee82fc85f12d8705e787a8
This refactor makes the field a bit more type-safe.
This also better reflects the meaning and use.
This is required in the next commit.
…oken_timestamp faaea78 refactor: Use current_time over redundant call to Now() (MarcoFalke) 3333c50 refactor: Use NodeClock::time_point for m_addr_token_timestamp (MarcoFalke) fa55723 move-only: Extract ProcessAddrs() helper (MarcoFalke) Pull request description: It is a bit confusing to have some code use the deprecated `GetTime`, which returns a duration and not a time point, and other code to use `NodeClock` time points. Fix one place `m_addr_token_timestamp` to use `NodeClock::time_point`. Also: * Extract a `ProcessAddrs` helper, similar to the other `Process*()` helpers, to cut down the `ProcessMessage` with a massive scope. * Rename the confusing `current_a_time` to `now_seconds`. (The `a` in this context refers to the removed "adjusted" time, see commit fadd8b2, which removed adjusted time here) ACKs for top commit: l0rinc: ACK faaea78 ajtowns: reACK faaea78 sedited: Re-ACK faaea78 Tree-SHA512: 67ad13e9d7b88e08e3d723e6b7cd598b38df2a004f5c2338b24f2992e25ae9d8fb8e5325c9c94171e551fe86d87e3e3ec1fe6baae64edbf6b5c125f408ee64e4
This refactor is a follow-up to commit eeeeb2a and does not change any behavior. However, it is nice to know that no global mocktime leaks from the fuzz init step to the first fuzz input, or from one fuzz input execution to the next. With the clock context, the global is re-set at the end of the context.
1. Use `build_os` instead of `host_os` for native packages. 2. `XCODE_VERSION` is available only for `darwin` hosts. Therefore, simply disable the Xcode version check for `native_qt`.
…/*` output a1f22a0 test: Suppress another unsolicited `mock_process/*` output (Hennadii Stepanov) Pull request description: This is a follow-up to bitcoin#33929. The[ `mock_process/*`](https://github.com/bitcoin/bitcoin/blob/390e7d61bd531505bb3d13f38316c282b85ed1dd/src/test/mock_process.cpp#L10) test cases, which serve as helpers for [`system_tests`](https://github.com/bitcoin/bitcoin/blob/390e7d61bd531505bb3d13f38316c282b85ed1dd/src/test/system_tests.cpp#L23) rather than actual tests, are invoked in a way that suppresses unsolicited output to stdout or stderr to keep the test results reproducible. However, in debug builds, the Windows CRT still prints false-positive memory leak dumps to stderr. This PR handles this specific case and documents the other suppressions. ACKs for top commit: maflcko: lgtm ACK a1f22a0 sedited: ACK a1f22a0 Tree-SHA512: 480f0f74ce50b6fb315bfeb6f6d3a4a4791557a6177aa995197c002bf871f878573df0f313344206f82e4d54c7a62d7cea2fdcb8cd96475b4b6cc7d4ffaf0538
This removes the need to patch vcpkg source code to skip debug builds for dependencies.
This makes `threadsafety.h` agnostic to the actual implementation.
7262adb4b4 Merge bitcoin-core/secp256k1#1841: gha: Bump deprecated GHA workflow dependencies c5cd9d6d9a gha: Bump deprecated GHA workflow dependencies 95b702de34 Merge bitcoin-core/secp256k1#1839: ecdsa: VERIFY_CHECK result of _fe_set_b32_limit 634215f3fc Merge bitcoin-core/secp256k1#1837: tests: Fix function pointer initialization C89 error in ellswift tests 43fca0ff55 ecdsa: VERIFY_CHECK result of _fe_set_b32_limit b84635ed3b tests: Fix C89 function pointer initialization in ellswift tests ffc25a2731 Merge bitcoin-core/secp256k1#1834: ecmult: Document and test ng=NULL in ecmult 3a403639dc eckey: Call ecmult with NULL instead of zero scalar 7e68c0c88b ecmult: Document and test ng=NULL in ecmult 1aafe15139 Merge bitcoin-core/secp256k1#1777: Make SHA256 compression runtime pluggable b9cb1cbfd7 Merge bitcoin-core/secp256k1#1824: util: introduce and use `ARRAY_SIZE` macro 4d92a083bc sha256: speed up writes using multi-block compression 0753f8b909 Add API to override SHA256 compression at runtime fdb6a91a5e Introduce hash context to support pluggable SHA256 compression c0a2aba088 Merge bitcoin-core/secp256k1#1811: bench: Update help functions in bench and bench_internal 10f546a2c0 Merge bitcoin-core/secp256k1#1832: testrand: Remove testrand_finish 8d0eda07e9 testrand: Remove testrand_finish 95e6815843 Merge bitcoin-core/secp256k1#1825: hash: remove redundant `secp256k1_sha256_initialize` in tagged hash midstate functions f48b1bfa5d hash: add midstate initializer and use it for tagged hashes 3019186a6d Merge bitcoin-core/secp256k1#1829: ci: Fix leftover use of old ECMULTGENPRECISION 79e9f25237 ci: Fix leftover use of old ECMULTGENPRECISION dfe042feb2 Merge bitcoin-core/secp256k1#1828: Revert "ci, docker: Fix LLVM repository signature failure" 76e92cfeea Revert "ci, docker: Fix LLVM repository signature failure" ac561601b8 Merge bitcoin-core/secp256k1#1760: cmake: Add dynamic test discovery to improve parallelism c7a7f732bd Merge bitcoin-core/secp256k1#1821: ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 921b9711ea util: introduce and use `ARRAY_SIZE` macro b99a94c382 Add tests for bad scalar inputs in ellswift XDH 307b49f1b9 ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 322d0a4358 Merge bitcoin-core/secp256k1#1823: ci: Load Docker image by ID from builder step ed02466d3f ci: Load Docker image by ID from builder step c49c9be504 bench: Update help functions in bench and bench_internal 1d146ac3ed Merge bitcoin-core/secp256k1#1819: tests: Improve secp256k1_scalar_check_overflow tests (Issue bitcoin#1812) f47bbc07f0 test: add unit tests for secp256k1_scalar_check_overflow d071aa56d5 Merge bitcoin-core/secp256k1#1815: refactor: remove unnecessary `malloc` result casts 99ab4a105e Merge bitcoin-core/secp256k1#1817: ci: Disable Docker build summary generation c5da3bde9c Merge bitcoin-core/secp256k1#1818: ci: Enforce base-10 evaluation 97de5120cf Merge bitcoin-core/secp256k1#1804: test: show both CMake and Autotools usage for ctime_tests 4fb7ccf5d4 ci: Enforce base-10 evaluation 3ae72e7867 ci: Disable Docker build summary generation 97b3c47849 refactor: remove unnecessary `malloc` result casts 1bc74a22f8 test: show both Autotools and CMake usage for ctime_tests 8354618e02 cmake: Set `LABELS` property for tests 29f26ec3cf cmake: Integrate DiscoverTests and normalize test names f95b263f23 cmake: Add DiscoverTests module 4ac651144b cmake, refactor: Deduplicate test-related code git-subtree-dir: src/secp256k1 git-subtree-split: 7262adb4b40074201fb30847035a82b8d742f350
…ations 0e712b3 Make DynSock accepted sockets queue optional, with precise lifetime (Matthew Zipkin) 3de02ab util/test: Add string_view constructor to LineReader and remove StringToBuffer (Matthew Zipkin) b0ca400 string: replace AsciiCaseInsensitiveKeyEqual with CaseInsensitiveEqual (Matthew Zipkin) 8172099 util: get number of bytes consumed from buffer by LineReader (Matthew Zipkin) Pull request description: This is a follow-up to bitcoin#34242 and is the first few commits of bitcoin#32061 As review and refinement of the replacement HTTP server progresses, some new utilities were needed and added. This PR updates those utilities as work continues on bitcoin#32061. ### LineReader In order to enforce strict limits on the total size of headers in HTTPRequest, we add a method to `LineReader` to give us the total amount of data that has been read from the buffer so far. See bitcoin#32061 (comment) ### CaseInsensitiveEqual HTTP headers are case-insensitive. An early version of bitcoin#32061 used an unordered_map for this and therefore we needed a comparator struct. However that unordered_map was replaced by a simpler `std::vector` of `std::pair` so we can remove the struct and use methods that already exist in the codebase. ### StringToBytes `StringToBuffer` was introduced in bitcoin#34242 to test LineReader but review of bitcoin#32061 indicated that it would be more optimal to return a span of bytes instead of a vector. See bitcoin#32061 (comment) ### Split DynSock constructor for two usecases: listening / accepting sockets See bitcoin#32061 (comment). DynSock was introduced in bitcoin#30988 and is not used anywhere in master yet. If it's used as a listening socket, it provides connected sockets. If it's used as a connected socket, it provides I/O pipes. By making the queue of connected sockets optional we can clean up the ownership / lifetime if the class members. ACKs for top commit: fjahr: Code review ACK 0e712b3 vasild: ACK 0e712b3 Tree-SHA512: 234c79a00c03cb3952dce2a3c5e59859bd0cbfc5f0a552ad2065e998320a12b533b06adbe294745c690a9e19c2f5f79bca3aa5a44342ee1820037342799566f2
…plementation 242b0eb btcsignals: use a single shared_ptr for liveness and callback (Cory Fields) b12f43a signals: remove boost::signals2 from depends and vcpkg (Cory Fields) a4b1607 signals: remove boost::signals2 mentions in linters and docs (Cory Fields) 375397e signals: remove boost includes where possible (Cory Fields) 091736a signals: re-add forward-declares to interface headers (Cory Fields) 9958f4f Revert "signals: Temporarily add boost headers to bitcoind and bitcoin-node builds" (Cory Fields) 34eabd7 signals: remove boost compatibility guards (Cory Fields) e60a0b9 signals: Add a simplified boost-compatible implementation (Cory Fields) 63c68e2 signals: add signals tests (Cory Fields) edc2978 signals: use an alias for the boost::signals2 namespace (Cory Fields) 9ade392 signals: remove forward-declare for signals (Cory Fields) 037e58b signals: use forwarding header for boost signals (Cory Fields) 2150153 signals: Temporarily add boost headers to bitcoind and bitcoin-node builds (Cory Fields) fd5e9d9 signals: Use a lambda to avoid connecting a signal to another signal (Cory Fields) Pull request description: This drops our dependency on `boost::signals2`, leaving `boost::multi_index` as the only remaining boost dependency for bitcoind. `boost::signals2` is a complex beast, but we only use a small portion of it. Namely: it's a way for multiple subscribers to connect to the same event, and the ability to later disconnect individual subscribers from that event. `btcsignals` adheres to the subset of the `boost::signals2` API that we currently use, and thus is a drop-in replacement. Rather than implementing a complex `slot` tracking class that we never used anyway (and which was much more useful in the days before std::function existed), callbacks are simply wrapped directly in `std::function`s. The new tests work with either `boost::signals2` or the new `btcsignals` implementation. Reviewers can verify functional equivalency by running the tests in the commit that introduces them against `boost::signals2`, then again with `btcsignals`. The majority of the commits in this PR are preparation and cleanup. Once `boost::signals2` is no longer needed, it is removed from depends. Additionally, a few CMake targets no longer need boost includes as they were previously only required for signals. I think this is actually pretty straightforward to review. I kept things simple, including keeping types unmovable/uncopyable where possible rather than trying to define those semantics. In doing so, the new implementation has even fewer type requirements than boost, which I believe is due to a boost bug. I've opened a PR upstream for that to attempt to maintain parity between the implementations. See individual commits for more details. Closes bitcoin#26442. ACKs for top commit: fjahr: Code review ACK 242b0eb maflcko: re-review ACK 242b0eb 🎯 w0xlt: reACK 242b0eb Tree-SHA512: 9a472afa4f655624fa44493774a63b57509ad30fb61bf1d89b6d0b52000cb9a1409a5b8d515a99c76e0b26b2437c30508206c29a7dd44ea96eb1979d572cd4d4
It is best if the internal addrman database is not modified with information coming from private broadcast connections because that information can potentially later be sent via other connections. Co-authored-by: Greg Sanders <gsanders87@gmail.com> Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
ba01b00 refactor: use for loops in FindMostWorkChain (stratospher) aa0eef7 test: add InvalidateBlock/ReconsiderBlock asymmetry test (stratospher) 1b0b3e2 validation: remove redundant marking in FindMostWorkChain (stratospher) Pull request description: recent PRs like bitcoin#31405, bitcoin#30666 mark all `m_block_index` descendants as invalid immediately whenever an invalid block is encountered in `SetBlockFailureFlags`. so by the time we reach `FindMostWorkChain`, the block in `setBlockIndexCandidates` already has `BLOCK_FAILED_VALID` set on it - not just on its ancestor. this means `pindexTest = pindexFailed` whenever `fFailedChain` fires, and the inner `while (pindexTest != pindexFailed)` loop body is never reached! I think we can remove it but I've just replaced it with `Assume` in this PR for safety + good to document this invariant in case the code changes in future. (noticed by @ stickies-v in bitcoin#32950 (comment)) the second commit is unrelated and adds a unit test for the situation in bitcoin#32173 ACKs for top commit: fjahr: re-ACK ba01b00 optout21: crACK ba01b00 w0xlt: ACK ba01b00 ryanofsky: Code review ACK ba01b00, just tweaking comment and for loop condition since last review. Tree-SHA512: a8be3c30b1c41b76690d16d850e87e9e71fa6a1ecaa8b90ec997ffee1aace48b336a7009a480cd016103759d79c964b3d761a13ae936523808b2930beb68dae5
2af003a test: Use BasicTestingSetup when TestingSetup is not necessary (Hodlinator) 9ee7770 refactor(test): Only specify TestChain100Setup in test cases (Hodlinator) Pull request description: Improves run-times (4 warmups + 60 runs each): | test | improvement | |:---|---:| | checkqueue_tests | 1.12 | | merkle_tests | 1.14 | | orphanage_tests | 1.48 | | prevector_tests | 1.07 | | rbf_tests | 1.01 | | validation_tests | 1.60 | --- Removed overhead is barely measurable on full test suite runtime sequential `test_bitcoin`or parallel `ctest --test-dir build`. --- Initial version of PR also extracted a `DataDirTestingSetup` from `BasicTestingSetup`, making the latter not use the disk, but the added complexity didn't deliver sufficient speedups to clearly justify itself (at least not while running on NVMe). --- Follow-up to bitcoin#34562 (comment) Arguably in a similar vein as bitcoin#22086. ACKs for top commit: l0rinc: ACK 2af003a achow101: ACK 2af003a ryanofsky: Code review ACK 2af003a. Changes seem good: switching to minimum required test fixtures and avoiding expensive `TestChain100Setup` usage for whole fixtures. w0xlt: ACK 2af003a Tree-SHA512: 56eba7595647447fd1940f205dac37605ae4b88f4947d542f1fc1c6c66791a7dbde244d4943f699dc11463e805725991d9d4db7f9be8c7f4d7054ca9dc2d79e1
… broadcast connections 1ed1a12 net_processing: don't modify addrman for private broadcast connections (Vasil Dimov) Pull request description: It is best if the internal addrman database is not modified with information coming from private broadcast connections because that information can potentially later be sent via other connections. instagibbs suggested this change, thank you! ACKs for top commit: l0rinc: code review ACK 1ed1a12 instagibbs: ACK 1ed1a12 achow101: ACK 1ed1a12 andrewtoth: ACK 1ed1a12 danielabrozzoni: tACK 1ed1a12 Tree-SHA512: 13845778445e56e3015a569f3b4165a749011e9dd67dcab38e960a368a0f5d3822173af5e3cb950998326549a021d1c8df644ce6d761cd46dd7597106b9ceec9
…ests 858a0a9 test: Add SRD maximum weight tests (Murch) fe9f53b test: Add SRD success tests (Murch) 2840f04 test: Rework SRD insufficient balance test (Murch) 64ab974 Test: Add new minimum to tested feerates (Murch) 65900f8 test: Init coin selection params with feerate (Murch) Pull request description: This transfers the tests from the old coin selection test framework that was based on ignoring fees to the new coin selection framework that tests the coin selection algorithms with realistic feerates and fees. The PR also includes a minor improvement of the test framework to allow the CoinSelectionParams used by the tests to be initialized with other feerates, and adds some feerates around the new minimum feerate to the tested feerates. ACKs for top commit: achow101: ACK 858a0a9 w0xlt: ACK 858a0a9 brunoerg: reACK 858a0a9 Tree-SHA512: cbd7ed3169d225a0b0f751481ca936a10939ff899c345e9469821d46083b04e835d164c50a72f18851544314611c3d596eb4956c1d86903c22e8b4996b8eb861
…=0.0.0.0:port bb00fd2 test: use dynamic ports and add coverage in feature_bind_port_discover (b-l-u-e) 4f19508 test: dont connect nodes in feature_bind_port_discover (b-l-u-e) b8827ce net: Fix Discover() not running when using -bind=0.0.0.0:port (b-l-u-e) Pull request description: This PR fixes two related issues with address discovery when using explicit bind addresses in Bitcoin Core: - bitcoin#31293 - bitcoin#31336 When using `-bind=0.0.0.0:port` (or `-bind=::`), the `Discover()` function was not being executed because the code only checked the `bind_on_any` flag. This led to two problems: - The node would not discover its own local addresses if an explicit "any" bind was used. - The functional test `feature_bind_port_discover.py` would fail, as it expects local addresses to be discovered in these cases. This PR: 1. Checks both `bind_on_any` and any bind addresses using `IsBindAny()` Ensures `Discover()` runs when binding to 0.0.0.0 or ::, even if specified explicitly. 2. Ensures correct address discovery The node will now discover its own addresses when using explicit "any" binds, matching user expectations and fixing the test. 3. Maintains backward compatibility The semantic meaning of `bind_on_any` is preserved as defined in `net.h`: > "True if the user did not specify -bind= or -whitebind= and thus we should bind on 0.0.0.0 (IPv4) and :: (IPv6)" 4. Updates the test to use dynamic ports The functional test `feature_bind_port_discover.py` is updated to use dynamic ports instead of hardcoded ones, improving reliability. References - Implementation follows the approach proposed in my [review comment on bitcoin#31492](bitcoin#31492 (comment)). Closes bitcoin#31293 The bitcoin#31336 has the overall test failure and requires both this PR and bitcoin#33362 to be fully resolved. ACKs for top commit: NicolaLS: tested ACK bb00fd2 pinheadmz: ACK bb00fd2 vasild: ACK bb00fd2 achow101: ACK bb00fd2 Tree-SHA512: 6f1b0b29cc539e4b49b3594f9ad70be90cfff28e31497a8b85e11d8a2509faaa8ca803e542ea3280588ece5a065c4c54193cec87cad221f1abae34d8b13cfdc5
faad08e test: Use NodeClockContext in more tests (MarcoFalke) fa8fe09 fuzz: Use NodeClockContext (MarcoFalke) fa9f434 test: Allow time_point in boost checks (MarcoFalke) Pull request description: Currently mocktime is written to a global, which may leak between sub-tests (albeit some tests try to reset the mocktime on a best-effort basis). Also, when advancing it, one has to keep a counter variable around. Fix both issues by using the recently added `NodeClockContext`, which resets the mocktime once it goes out of scope. Also, it has a method to advance the mocktime by a delta. ACKs for top commit: achow101: ACK faad08e seduless: Tested ACK faad08e frankomosh: Tested ACK faad08e. Ran all relevant tests, all clean. Also verified that the default-constructor call sites in orphanage_tests and addrman_tests behave identically to the explicit `{Now<NodeSeconds>()}` form. ryanofsky: Code review ACK faad08e but had a question about dropping +1 in one test below. Tree-SHA512: bd56931970eed02bfcf3f3593ef64a61a8a1d8cc8adf190d6903b35df0fd7e6d865678c7d5bd23ce53d074cb2cf53a0a19212fdeb593b047dac5561859bc86b0
…r-notes ea893cf doc: fix typo 'parlor' to 'parlance' in developer-notes (ArvinFarrelP) Pull request description: Fix a small typo in doc/developer-notes.md. Replace "parlor" with "parlance" to use the correct term in the context of C++ terminology. ACKs for top commit: maflcko: lgtm ACK ea893cf l0rinc: ACK ea893cf Tree-SHA512: 5437e0eb1b40aa1eafe8a0482350ed6259ec3492f64e7ba8d046358a3dd9cf95bfc38825b781350fd321c453cc23dddb6a14fe7f5087e56c77a17e3d3e233de4
dfd54c9 Squashed 'src/secp256k1/' changes from 57315a6985..7262adb4b4 (fanquake) Pull request description: bitcoin-core/secp256k1#1760 was merged upstream, which improves test parallelism, and reduces overall runtime. Our longest running tests are secp256k1 tests (`secp256k1_tests`), so we might want to pull this down, to speedup all of our ctest invocations. The new upstream code increases output verbosity, i.e the test list is now ~330, and we have output like: ```bash Label Time Summary: secp256k1_exhaustive = 19.52 sec*proc (1 test) secp256k1_noverify_tests = 45.65 sec*proc (91 tests) secp256k1_tests = 90.55 sec*proc (91 tests) ``` maybe we can/should mute that somewhat? ACKs for top commit: hebasto: ACK b7f9178. Tree-SHA512: 098a8b54d3a489e6ad7866f4e5152a5bc6a0e1da69b2a84d8795423e64faa42772cbcb194feac228a547869d2b3be2198aaf96a5e1bb7d45a2e385fd5e78bafd
…o `assert_equal` 3fd68a9 scripted-diff: replace remaining Python test equality asserts (Lőrinc) 301b1d7 test: add missing `assert_equal` imports (Lőrinc) 06a4176 test: convert truthy asserts in `wallet_miniscript` and `rpc_psbt` (Lőrinc) d9a3cf2 test: convert simple equality asserts in excluded files (Lőrinc) 23c06d4 test: convert equality asserts with comments or special chars (Lőrinc) dcd90fb test: prep manual equality assert conversions (Lőrinc) 4f4516e test: split equality asserts joined by `and` (Lőrinc) 76a5570 test: use `in` for two-value equality asserts (Lőrinc) Pull request description: ### Problem Plain `assert x == y` is a poor fit for this test framework, `assert_equal()` gives more useful failure output and keeps equality checks consistent with the rest of the functional tests. ### Design A simple scripted diff cannot safely rewrite all of them because many files use `==` inside larger expressions, such as chained conditions, comprehensions, and other compound assertions. That makes a one-shot mechanical conversion either incorrect or harder to review. ### Fix This series first rewrites the non-mechanical cases into standalone assertions so patterns are easier to identify, then applies a scripted diff to the remaining cases that are safe to convert mechanically. Partially fixes bitcoin#23119 by adjusting the `==` case only (which is similar to the `BOOST` alternatives so it's expected to be uncontroversial). ACKs for top commit: maflcko: review ACK 3fd68a9 🚆 rkrux: lgtm ACK 3fd68a9 theStack: ACK 3fd68a9 Tree-SHA512: 6950ffa044db72d6235305f4c0247254e7e8f57ee1c8300e553953963914a6360ca71569fe315ecb333cf0e62f78b3a24603717f64229783761f8c1b5958fc12
…erface 8783cc8 refactor: inline `CCoinsViewBacked` implementation (Lőrinc) 86296f2 coins: make `CCoinsView` methods pure virtual (Lőrinc) b637566 coins: add explicit `CoinsViewEmpty` noop backend (Lőrinc) 90c635c fuzz: keep backend assertions aligned to active backend (Lőrinc) a9f92e3 refactor: normalize CCoinsView whitespace and signatures (Lőrinc) 38a99f3 scripted-diff: normalize `CCoinsView` naming (Lőrinc) 06172ef refactor: rename `hashBlock` to `m_block_hash` to avoid shadowing (Lőrinc) Pull request description: ### Problem `CCoinsView` is the coins view interface, but historically it also provided built-in no-op behavior: * It provided default no-op implementations (returning `std::nullopt`, `uint256()`, `false`, or `nullptr`) instead of being pure virtual. * Callers could instantiate a bare `CCoinsView` and get silent no-op behavior. * Mixing the interface definition with a built-in dummy implementation blurred the abstraction boundary. ### Context This is part of the ongoing coins caching cleanup in bitcoin#34280. ### Fix This PR separates the interface from no-op behavior and makes `CCoinsView` pure virtual in incremental steps: * Add `CoinsViewEmpty` as an explicit no-op coins view for tests, benchmarks, and temporary backends. * Replace direct bare-`CCoinsView` test and dummy instantiations with `CoinsViewEmpty`. * Make all `CCoinsView` methods pure virtual (`PeekCoin`, `GetCoin`, `HaveCoin`, `GetBestBlock`, `GetHeadBlocks`, `BatchWrite`, `Cursor`, `EstimateSize`). * Remove the legacy default implementations from `coins.cpp`. * Update fuzz and dummy backends to use `CoinsViewEmpty` explicitly. ACKs for top commit: w0xlt: reACK 8783cc8 ryanofsky: Code review ACK 8783cc8. Just updated comments and variable name since last review. The fuzz test code is much clearer now IMO andrewtoth-exo: ACK 8783cc8 ajtowns: ACK 8783cc8 Tree-SHA512: cfc831578aa309788c1b5dafbfecca3de388cc4215534c3f3df24f90d7770ed37b1fd7aa134df91d611d0a1ca75929accb98d5ed7df7b52851c259e04f08e4a3
…bevent behavior 422ca21 test: ensure HTTP server enforces limits on headers and body size (Matthew Zipkin) 0c1a07e test: ensure HTTP server timeout is not caused by a delayed response (Matthew Zipkin) f06de5c test: clean up and modernize interface_http (Matthew Zipkin) Pull request description: This is a follow-up to bitcoin#32408 and a new prerequisite for bitcoin#32061 in response to a few review comments there. ### New test: `check_server_busy_idle_timeout()` In bitcoin#32061 (comment) it is pointed out that the idle timeout set by `-rpcservertimeout` could disconnect a client unfairly if it was the server that was taking too long to process a response. That misbehavior was confirmed and bitcoin#32061 was updated to match current libevent behavior. This PR asserts the current libevent behavior by adding another test using RPC `waitforblock` past the `-rpcservertimeout` value and then verifying that the HTTP connection is still open. ### Expanded tests: `check_excessive_request_size()` and `check_chunked_transfer()` bitcoin#32061 (comment) made me realize that I was not testing HTTPRequest body size at all, and that headers size was being tested one line at a time but not in total. The libevent server does both things so that behavior is asserted in these expanded tests, and the mechanism in bitcoin#32061 was improved to match. ### Clean up `interface_http.py` Since I am extending this test module again I refactored the monolithic test to resemble the current functional test style in the repository, de-duplicating HTTP connection code with a helper class and separating individual test cases into functions. ACKs for top commit: fjahr: ACK 422ca21 Bortlesboat: re-ACK 422ca21. Checked out the rebased branch and ran interface_http.py locally, passes clean. Went through all three commits -- the helper class deduplication reads well and check_server_busy_idle_timeout is a nice addition from the bitcoin#32061 discussion. One minor note inline. hodlinator: Concept ACK 422ca21 theStack: ACK 422ca21 Tree-SHA512: 1165c9c49c8b1ae5a40a15e1d0f8275bb4d5e08a5eea7ae8b9d900bb34a85b29ba89c728b5e0866fbf289dd49aa5ece0af63e410e64aee70c89a19759c5ea3ff
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Add `btck_block_header_to_bytes` serialization method to serialize a `btck_BlockHeader` into an 80-byte buffer using `SpanWriter` to ensure zero-allocation serialization.
Add return type check for `ToBytes()` which should be convertible to `std::span<const std::byte>`.
The serfloat module is only used for fee estimation, which is not included in the bitcoinkernel library, so the dependency can be removed.
…hainstate.py Apply the timeout factor inside the add_block function. Also, force named args for the two expected strings. Also, add trailing comma for style.
49895b9 kernel: build: remove unused serfloat dependency (Sebastian Falbesoner) Pull request description: The serfloat module (more concretely, its two functions `{Encode,Decode}Double`) is only used for [fee estimation](https://github.com/bitcoin/bitcoin/blob/7844a2f08303edb665e4292d002a826f24e62832/src/policy/fees/block_policy_estimator.cpp#L21), which is not included in the bitcoinkernel library, so the linking dependency can be removed. (Not terribly important in practice, but: this reduces the static library size by ~1.5 KB (~2.7 KB unstripped) on my arm64 Linux machine.) ACKs for top commit: davidgumberg: ACK bitcoin@49895b9 stickies-v: ACK 49895b9 Tree-SHA512: c3b4feec55cdc7476f54185db95c94e7365f5feadf09d3740d51dce32a1ceeef4d38ecf5c8cf7ca4ed326a12dab386bb104da978ff06bdf739889b2cfa81603d
…ale in tool_bitcoin_chainstate.py fa02eb8 test: Add missing self.options.timeout_factor scale in tool_bitcoin_chainstate.py (MarcoFalke) Pull request description: Without the factor, the test may fail when run cross-arch, possibly with sanitizers. E.g. in qemu-aarch64 in docker in a x86-VM. ACKs for top commit: fanquake: ACK fa02eb8 Tree-SHA512: 7dbf492de7478bcc95a62576499947fa83031c43496700657014aa9b29f6665c2a226b71287b71b8cd7a5240036c00f069f12ab231539e3387fe86ca298d6933
…eader API 577a3e7 test: Add check for return type in `HasToBytes` concept (yuvicc) 1ad5512 kernel: Add Block Header serialization method (yuvicc) 8666262 Add `SpanWriter` class for zero-allocation stream writing (yuvicc) Pull request description: This adds serialization for `btck_BlockHeader` API. Also, updated the `CheckHandle` to compare the byte content instead of size. The changes here is done in two commits. First commit adds the `SpanWriter` class and next one moves the block header serialization to `SpanWriter`. See commit message for more details. Follow-up to bitcoin#33822 . ACKs for top commit: stickies-v: re-ACK 577a3e7 alexanderwiederin: ACK bitcoin@577a3e7 theStack: Code-review ACK 577a3e7 w0xlt: ACK 577a3e7 Tree-SHA512: 1eda5b204588ccb23e9357f68c5529474e7d248736a371c47d8db71ba6ca95e121869514478ad7a519d190e4c30725f64fd1ef4dd9f97d2627dc4441e51458e0
19e99be guix: remove riscv exclusion from symbol check (fanquake) 47b7a9f guix: binutils 2.46.0 (fanquake) Pull request description: Switching to binutils `2.46.0` fixes the spurious exported symbols (2.45.1 was still broken). The relevant upstream change is https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=9e10fcf71c1101fb6422d0f52de5e615ed8df71d. Fixes bitcoin#28095. ACKs for top commit: hebasto: re-ACK 19e99be, only rebased and properly adjusted since my recent [review](bitcoin#34757 (review)). Tree-SHA512: 5673e8df8e2297e41c3f50fbe87ee506684a7dd7e8be27fc5613853ace9732d92616c10870614f6b196ca71a581470eb6f432cfd102e1fce58bbaf18c599342e
….2.90 f1e14df depends: remove workaround for Make older than 4.2.90 (fanquake) Pull request description: This was introduced for distros shipping older `make`, such as Ubuntu `20.04` (`4.2.1`). It's likely that any distros being used for Darwin and Windows cross compilation, are shipping a newer make at this point. ACKs for top commit: hebasto: ACK f1e14df, I have reviewed the code and it looks OK. Tree-SHA512: bb5d785e4804f0b0d51c5abba31ee4537cf04247801edef51a5da728c7df7fff1189625d222f91b8f5896f9ec71dc8dbd11614a69e13e0dcad6017cab7dd5874
- CONTRIBUTING.md: "Valid areas as" -> "Valid areas are" - doc/i2p.md: remove double space after period - doc/bitcoin-conf.md: "some negating some lists" -> "some negating lists" - doc/files.md: missing space after comma in "macOS,the"
bdc8e49 doc: fix typos and formatting in CONTRIBUTING, i2p, bitcoin-conf, files (Guillermo Fernandes) Pull request description: Fix four small documentation issues found in actively maintained files: - `CONTRIBUTING.md`: "Valid areas as:" → "Valid areas are:" (grammar) - `doc/i2p.md`: remove double space after period - `doc/bitcoin-conf.md`: "some negating some lists" → "some negating lists" (duplicate word) - `doc/files.md`: missing space after comma in `**macOS**,the` ACKs for top commit: maflcko: lgtm ACK bdc8e49 NellyNakhero: ACK bdc8e49 Tree-SHA512: 15a5e284d517f7311a8c7c02a17292504a9eeafbaa81dc12a63a22fa1d49122e437e5647d9d0fbf397b5d150d0586632daccf0d2a7e7775a9bfd38083f0dfdcd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
iwyu compat