More-like-weekly cherry-pick#33
Merged
Merged
Conversation
fa99a3c ci: Enable pipefail in 03_test_script.sh (MarcoFalke) Pull request description: The CI script is problematic, because it is written in Bash, without pipefail enabled. Thus, some failures are silently ignored. Enabling pipefail is a bit tedious, because: * The IWYU task has no (`--verbose`) ccache output, so the pipe fails after `grep` [1]. Also, right now on master, the if silently skips: `ci/test/03_test_script.sh: line 122: [: : integer expression expected`. * The Alpine task has `Hits:` twice in the output, so the pipe fails after `head -1` [2] Not sure what the easiest way to fix this would be. Some options are: * Just use `tail -1` and `0` as fallback: `hit_rate=$(ccache --show-stats | grep "Hits:" | tail -1 | sed 's/.*(\(.*\)%).*/\1/' || echo "0")` * Properly parse, using Python and `--print-stats` (this pull) [1] ``` + ccache --version + head -n 1 ccache version 4.11.2 + ccache --show-stats --verbose Cache directory: /home/admin/actions-runner/_work/_temp/ccache_dir Config file: /home/admin/actions-runner/_work/_temp/ccache_dir/ccache.conf System config file: /etc/ccache.conf Stats updated: Tue Feb 17 08:40:20 2026 Local storage: Cache size (GB): 0.0 / 2.0 ( 0.00%) Files: 0 Hits: 0 Misses: 0 Reads: 0 Writes: 0 ++ ccache --show-stats ++ grep Hits: ++ head -1 ++ sed 's/.*(\(.*\)%).*/\1/' + hit_rate= Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', 'f5e8f319c22101ada5be9d4c5fd7d883ce37b830e86ec64627cb7d2b96749053', '/home/admin/actions-runner/_work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 1. Error: Process completed with exit code 1. ``` [2] ``` + ccache --version + head -n 1 ccache version 4.12.1 + ccache --show-stats --verbose Cache directory: /home/admin/actions-runner/_work/_temp/ccache_dir Config file: /home/admin/actions-runner/_work/_temp/ccache_dir/ccache.conf System config file: /etc/ccache.conf Stats updated: Tue Feb 17 08:40:35 2026 Cacheable calls: 873 / 873 (100.0%) Hits: 846 / 873 (96.91%) Direct: 822 / 846 (97.16%) Preprocessed: 24 / 846 ( 2.84%) Misses: 27 / 873 ( 3.09%) Successful lookups: Direct: 822 / 873 (94.16%) Preprocessed: 24 / 51 (47.06%) Local storage: Cache size (GB): 2.0 / 2.0 (99.95%) Files: 2580 Cleanups: 13 Hits: 846 / 873 (96.91%) Misses: 27 / 873 ( 3.09%) Reads: 1772 Writes: 52 ++ ccache --show-stats ++ grep Hits: ++ head -1 ++ sed 's/.*(\(.*\)%).*/\1/' + hit_rate=96.91 Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', '272a66a48206f1f6096612e196127ce46ea4dbff5dc14be3a4a20c4ee523956f', '/home/admin/actions-runner/_work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 141. Error: Process completed with exit code 1. ``` ACKs for top commit: willcl-ark: ACK fa99a3c Tree-SHA512: e5b0ddad8f279bd48102543b0496fa2ecdfc6938d208078595a60b96680467a80504b21acdecd86204b82ce2770eede23e498f04c6a9cee59634f83d44cfe094 (cherry picked from commit 59918de)
…ndshake dfe5d6a fuzz: apply node context reset pattern to p2p_handshake (frankomosh) Pull request description: Follow-up to bitcoin#34302. Applies the node context reset pattern from fabf8d1 to `p2p_handshake`. Previous code pattern created local `AddrMan` and `node::Warnings` objects, and passed them to `PeerManager::make`. `connman` was left holding a dangling `reference_wrapper<AddrMan>` across iterations, since the local objects destruct at iteration end while `connman` is global. Like in fabf8d1 , reset and reinstall `node.addrman` and `node.peerman` on each iteration. This PR also removes `includes` made unused by this or prior refactors. ACKs for top commit: nervana21: tACK dfe5d6a maflcko: review ACK dfe5d6a 🦏 sedited: ACK dfe5d6a Tree-SHA512: 141ddec03c6d37f76a3b2d94701d18c851e85ea74e57716abb69ecc955d30371e342c6e267d2669ad853fe2d95fb77dd2fb506e4233ae3a88501d59ee1bbae30 (cherry picked from commit de92545)
… to developer notes 2e9fdcc doc: add feature deprecation and removal process to developer notes (Guillermo Fernandes) Pull request description: Closes bitcoin#31980 Adds a dedicated **"Feature deprecation and removal process"** section to `doc/developer-notes.md` covering the full deprecation lifecycle for all major feature categories. ## What's added **General principles** - Grace period is one major release (deprecated in N, removed in N+1) - Deprecation and removal both require release notes - Deprecated features should remain accessible via a re-enable flag during the grace period **Per-category guidance covering:** - RPC methods and fields (`-deprecatedrpc=<feature>` pattern, help text requirements, worked example pointing to bitcoin#31278) - Startup options (`LogWarning`/`InitWarning` on use, help text update) - REST interface (document in `doc/REST-interface.md`) - ZMQ (document in `doc/zmq.md`) - Wallet settings (defer to RPC or startup option process depending on exposure) This consolidates the process that currently exists only implicitly across PRs and issue discussions into one place for contributors to reference. ACKs for top commit: maflcko: lgtm ACK 2e9fdcc polespinasa: ACK 2e9fdcc stickies-v: ACK 2e9fdcc sedited: ACK 2e9fdcc Tree-SHA512: 1d43df410664a45f937bcbd250664f13379168ca90e3024bea506e21a88177e201dcb4fadade705735099e3b8aaa2102a3080ad005bffb3aecb8f08d530d4277 (cherry picked from commit 9c15022)
…ifests d846444 guix: Split manifest into build and codesign manifests (Hennadii Stepanov) 0b9e10a guix: Update `python-signapple` and wrap with OpenSSL paths (Hennadii Stepanov) Pull request description: This PR narrows the scope of the Guix environments to include only the minimum dependencies required for specific tasks, namely building and codesigning. ACKs for top commit: fanquake: ACK d846444 Tree-SHA512: f7b0dfc47e1c6c064738be9aeba69b8d553c7f61186b2c03fedf0a11015ab454cac45d6ee28bbabbbd53a3efabc230b77365edf9feb0d4a26c2805079389501d (cherry picked from commit e3f5c18)
c03107a ci: switch to GitHub cache for all runners (willcl-ark) Pull request description: Cirrus is winding down, and github now offers more than 10GB cache. Switch to GH cache for all runner-types. Simplify configure-docker action. ACKs for top commit: maflcko: review ACK c03107a 🚴 Tree-SHA512: b6111c7559a86eed8488a4b0775df812a303a99eed5c80297593936e61d1d5e2ce72fe2fa615625816672414e6947ac4d93b9fd2925522fba06417ea4711ce79 (cherry picked from commit f28cd75)
b4aeab2 to
715ed4d
Compare
faf6afd doc: Move mutex and thread section into guideline section (MarcoFalke) fa514ca doc: move-only Valgrind section (MarcoFalke) fa0202f doc: move-only Python section (MarcoFalke) fa37606 doc: Regroup clang-tidy rules (MarcoFalke) fa9c2dd doc: Fix to use lower-case anchors in links to C++ Core Guidelines (MarcoFalke) Pull request description: The anchors in isocpp links were recently broken, so fix them to point to the correct anchor. Also, move/regroup 3 sections while touching the file. ACKs for top commit: fanquake: ACK faf6afd sedited: ACK faf6afd Tree-SHA512: 0067eb23a6a8cccfaee5df0df347529f17db39473602fa51bc2f3e53c9709934bb25fca51f6ed58c4896437c890789f29facf54d15a7ebbbd247a2ebb1c0b5cd (cherry picked from commit 9767e80)
c0b5e0f to
0b4166e
Compare
…m runtime options 1e5d3b4 doc: add release note for mining option validation (Sjors Provoost) 0317f52 ci: enforce iwyu for touched files (Sjors Provoost) 8c58f63 refactor: have mining files include what they use (Sjors Provoost) 3bb6498 mining: store block create options in NodeContext (Sjors Provoost) 4637cd1 mining: reject invalid block create options (Sjors Provoost) 8daac1d mining: add block create option helpers (Sjors Provoost) 128da7c miner: add block_max_weight to BlockCreateOptions (Sjors Provoost) fa81e51 mining: parse block creation args in mining_args (Sjors Provoost) 0201660 mining: use interface for tests, bench and fuzzers (Sjors Provoost) 44082be interfaces: make Mining use const NodeContext (Sjors Provoost) d4368e0 move-only: add node/mining_types.h (Sjors Provoost) 6aeb1fb test: cover IPC blockmaxweight policy (Sjors Provoost) 63b23ea test: regression test for waitNext mining policy (Sjors Provoost) 24750f8 test: add createNewBlock failure helper (Sjors Provoost) 63ee9cd test: misc interface_ipc_mining.py improvements (Sjors Provoost) Pull request description: Although this PR is primarily a refactor, _there are behavior changes_ documented in the release note: - the IPC mining interface now rejects out-of-range block template options instead of silently clamping them; - startup now rejects `-blockmaxweight` values lower than `-blockreservedweight`, instead of allowing them to be clamped later. The interaction between node startup options like `-blockreservedweight` and runtime options, especially those passed via IPC, is confusing. They're combined in `BlockAssembler::Options`, which this PR gets rid of in favour of `BlockCreateOptions`. `BlockCreateOptions` is used by interface clients. As before, IPC clients have access to a safe / sane subset, whereas RPC and test code can use all fields. The same type is also used to store mining defaults parsed once during node startup in `NodeContext`. The maximum block weight setting (`block_max_weight`) is optional. When read from startup options it matches `-blockmaxweight`; when provided by callers it is a runtime override. `Merge()` fills unset fields from startup defaults while preserving caller-provided values. This all happens in commits `mining: add block create option helpers` and `mining: store block create options in NodeContext`, and requires some preparation to keep things easy to review. We get rid of `BlockAssembler::Options` but this is used in many tests. Since large churn is inevitable, we might as well switch all tests, bench and fuzzers over to the Mining interface. The `mining: use interface for tests, bench and fuzzers` commit does that, dramatically reducing direct use of `BlockAssembler`. Two exceptions are documented in the commit message. Because `test_block_validity` wasn't available via the interface and the block_assemble benchmark needs it, it's moved from `BlockAssembler::Options` to `BlockCreateOptions` (still not exposed via IPC). We need access to mining related structs from both the miner and node initialization code. To avoid having to pull in all of `BlockAssembler` for the latter, the `move-only: add node/mining_types.h` commit introduces `node/mining_types.h` and moves `BlockCreateOptions`, `BlockWaitOptions` and `BlockCheckOptions` there from `src/node/types.h`. I considered also moving `DEFAULT_BLOCK_MAX_WEIGHT`, `DEFAULT_BLOCK_RESERVED_WEIGHT`, `MINIMUM_BLOCK_RESERVED_WEIGHT` and `DEFAULT_BLOCK_MIN_TX_FEE` there from `policy.h`, since they are distinct from relay policy and not needed by the kernel. But this seems more appropriate for a follow-up and requires additional discussion. --- I kept variable renaming and other formatting changes to a minimum to ease review with `--color-moved=dimmed-zebra`. ## Commit summary Tests and test cleanup: - `test: misc interface_ipc_mining.py improvements` - `test: add assert_create_fails helper` - `test: regression test for waitNext mining policy` - `test: cover IPC blockmaxweight policy` Refactoring test/bench/fuzz callers: - `interfaces: make Mining use const NodeContext` - `mining: use interface for tests, bench and fuzzers` Moving mining interface types: - `move-only: add node/mining_types.h` Separating startup defaults from runtime options: - `mining: parse block creation args in mining_args`: adds `node/mining_args.{h,cpp}` and moves mining option parsing out of `init.cpp`, without storing the parsed values yet. - `miner: add block_max_weight to BlockCreateOptions`: moves the runtime maximum block weight setting into `BlockCreateOptions` as an optional value, so it can later be defaulted from startup args when unset. - `mining: add block create option helpers`: centralizes block template option defaulting and merging, removes `BlockAssembler::Options`, and preserves behavior except for dropping the `Specified ` prefix from startup option error messages. - `mining: reject invalid block create options`: checks typed `BlockCreateOptions` before block template creation, so invalid runtime options are rejected instead of silently clamped. Startup validation also rejects `-blockmaxweight` values lower than `-blockreservedweight`. - `mining: store block create options in NodeContext`: stores the startup mining options in `NodeContext` as `BlockCreateOptions`, so startup defaults and runtime overrides can be merged with the same option type. Include hygiene, CI and release note: - `refactor: have mining files include what they use` - `ci: enforce iwyu for touched files` - `doc: add release note for mining option validation` ACKs for top commit: w0xlt: reACK 1e5d3b4 sedited: ACK 1e5d3b4 ryanofsky: Code review ACK 1e5d3b4. Looks good, thanks for the updates! Tree-SHA512: 28c715023cb78f02775caa787b243c994bd0f8ce4559afc8db9301e93400ebbc74963626a4afe65ae15bcc16b9192d051a745839f4c804848d50746ea5a224b4 (cherry picked from commit a4157fc)
…ions 0774eaa util: Require integers for SaturatingAdd() and AdditionOverflow() (Hodlinator) a815e3e rpc: Correct type for tx_sigops (Hodlinator) Pull request description: * Correct copy-paste error in RPC code * Require proper integers for `SaturatingAdd()` and `AdditionOverflow()` in src/util/overflow.h These changes increase the type safety of the code and were done while exploring increasing the type-safety of `CAmount` (currently just a `typedef` of `int64_t`). The first commit has nothing to do with overflow but is along for the ride if reviewers agree. ACKs for top commit: maflcko: lgtm ACK 0774eaa winterrdog: ACK 0774eaa sedited: ACK 0774eaa Tree-SHA512: a245ad52cfd1c257151aea1a1ed4b6769415c1dddc7c405d8bbe71b9f3abc512a6d890a45cbc8381718be16274e337cc876e6e6da11dc35de71bea83bece6634 (cherry picked from commit 0687438)
…otes.md d5adb9d doc: fix doxygen links to threads in developer-notes.md (Matthew Zipkin) Pull request description: The "threads" section of `developer-notes.md` has links to anchor tags in the code generated by doxygen. As far as I can tell this was introduced in bitcoin#18645 and changes to this section of this document have continued the pattern. The problem is, the content at `https://doxygen.bitcoincore.org` gets re-rendered daily and those anchor tags are generated internally by doxygen, so they are all broken now. This PR adds doxygen syntax `\anchor XXXX` comments in the code where functions that run in these threads are defined, and then those stable, human-readable anchor tags are applied to the links in the doc. I have generated the doxygen output from this branch, hosted it on my own web server, and created a modified `developer-notes.md` with these anchor tags and my server as host for demonstration: https://gist.github.com/pinheadmz/ed3dda7d3c8d589e3989040519190b84#threads Just note when looking at this: - `main` is at the bottom of the html page so it might not look right at first - `initload` is a lambda inside `AppInitMain` so thats where doxygen renders the anchor ACKs for top commit: fanquake: ACK d5adb9d rkrux: lgtm ACK d5adb9d Tree-SHA512: c5517823a2d668b01318b3dae3d76fdd9db8a74d8c721aeb748e4f4a6cb56cb4d24e34b2590a41f8553992005cab368fca4ce322a4f204cec16ce338337ae9ee (cherry picked from commit 00af562)
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
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.
No description provided.