Skip to content

More-like-weekly cherry-pick#33

Merged
rustaceanrob merged 10 commits into
2140-dev:masterfrom
rustaceanrob:26-5-28-cp
May 28, 2026
Merged

More-like-weekly cherry-pick#33
rustaceanrob merged 10 commits into
2140-dev:masterfrom
rustaceanrob:26-5-28-cp

Conversation

@rustaceanrob
Copy link
Copy Markdown
Member

No description provided.

fanquake and others added 5 commits May 28, 2026 10:39
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)
@rustaceanrob rustaceanrob force-pushed the 26-5-28-cp branch 2 times, most recently from b4aeab2 to 715ed4d Compare May 28, 2026 13:00
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)
@rustaceanrob rustaceanrob force-pushed the 26-5-28-cp branch 3 times, most recently from c0b5e0f to 0b4166e Compare May 28, 2026 15:48
ryanofsky and others added 4 commits May 28, 2026 17:07
…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)
@rustaceanrob rustaceanrob merged commit de5a461 into 2140-dev:master May 28, 2026
19 checks passed
@rustaceanrob rustaceanrob deleted the 26-5-28-cp branch May 28, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants