📝 cedwies opened a pull request: "log: check fclose() results and report safely in logging.cpp"
(https://github.com/bitcoin/bitcoin/pull/33646)
`fclose()` can report write errors (for example if the disk is full or the filesystem has a problem). Right now, some `fclose()` calls in `src/logging.cpp` ignore the return value. This means errors might go unnoticed and log lines could be lost without warning.

## What this PR does:
- Add a small helper that prints `fclose()` errors to `stderr` (with path and errno).
- In shutdown: close `m_fileout` safely and report errors.
- In reopen: open the new file, swap it in, close the old one, a
...

✅ cedwies closed a pull request: "log: check fclose() results and report safely in logging.cpp"
(https://github.com/bitcoin/bitcoin/pull/33646)

📝 cedwies reopened a pull request: "log: check fclose() results and report safely in logging.cpp"
(https://github.com/bitcoin/bitcoin/pull/33646)
`fclose()` can report write errors (for example if the disk is full or the filesystem has a problem). Right now, some `fclose()` calls in `src/logging.cpp` ignore the return value. This means errors might go unnoticed and log lines could be lost without warning.

## What this PR does:
- Add a small helper that prints `fclose()` errors to `stderr` (with path and errno).
- In shutdown: close `m_fileout` safely and report errors.
- In reopen: open the new file, swap it in, close the old one, a
...

💬 paulxxmontoya33-hue commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3416801275)
> Hardware
>
> ```
> Darwin Chriss-MacBook-Pro.local 24.6.0 Darwin Kernel Version 24.6.0: Mon Jul 14 11:30:55 PDT 2025; root:xnu-11417.140.69~1/RELEASE_ARM64_T6031 arm64
> ```
>
>
> ### v29
>
> ```
> Bitcoin Core daemon version v29.0.0
> Copyright (C) 2009-2025 The Bitcoin Core developers
> ```
>
> ```
> $ time bitcoin-cli -regtest generatetoaddress 2000 $(bitcoin-cli -regtest getnewaddress)
> 0.01s user 0.01s system 0% cpu 1:56.41 total
> ```
>
> ### v30
>
> ```
> Bitcoin Core daemon
...

🤔 stringintech reviewed a pull request: "kernel: Introduce C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3351511583)
A few comments on CI jobs - might be missing some context as this is my first look at Core's CI setup.

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2440963037)
It seems any job that has both `RUN_FUNCTIONAL_TESTS` and `BUILD_UTIL_CHAINSTATE` enabled would run the `tool_bitcoin_chainstate.py` functional test. Currently the `No wallet, libbitcoinkernel` (Linux) and `Windows native, VS 2022` jobs run this test. Would it be useful to enable it for macOS too? (macOS native job already has functional tests enabled)

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2440887703)
I noticed the Windows cross build job builds the kernel library (`-DBUILD_KERNEL_LIB=ON`) but the macOS cross build job (in `00_setup_env_mac_cross.sh`) doesn't. Is this intentional?

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2441038796)
I noticed comparing the Windows native job (`Windows native, VS 2022`) and Windows cross job (`Linux->Windows cross`), only in the first one unit tests are executed. But we are building kernel unit tests only for the second one. It seems there is no job to run kernel unit tests on Windows?

💬 hebasto commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3416892977)
Here are the recent updates from the upstream projects:

1. Clang has [merged](https://github.com/llvm/llvm-project/commit/10be254587da24d56e2c6817b382beaca612b6c3) the fix and [backported](https://github.com/llvm/llvm-project/commit/570c4c9443387b756ed3e4cb94ca708841f2472a) it to 21.x.

2. The recent (2025-10-17) nightly release of [LLVM MinGW](https://github.com/mstorsjo/llvm-mingw/releases/nightly) (`llvm-mingw-nightly-ucrt-ubuntu-22.04-x86_64.tar.xz`, sha256:`0d7520a59724ca5dfb41fb8dc870
...

💬 hebasto commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3416903288)
> I think this only becomes a discussion if we are also using it for both architectures.

The LLVM MinGW successfully [builds](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18567774888) Bitcoin Core for both x86_64 and ARM64 architectures.

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2441132375)
It seems without the assertion, this just returns nullptr when `phashBlock` is null (which should never happen)? I am not sure how useful an assertion could be here though.

🤔 glozow reviewed a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3351431350)
Focused on reviewing the RBF logic in e6315c24326016cfaee5bd046e8b2e4e1088ac6b

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440892816)
fba5200d59af87db240a1b061f995c63b0ed5ee8 nit: Could be "(Removed)" to be consistent with the other rules we no longer implement?

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440839627)
in e6315c24326016cfaee5bd046e8b2e4e1088ac6b

Nice! We should also remove "All conflicting clusters (connected components of mempool transactions) must be clusters of up to size 2." from the package RBF rules in policy/packages.md

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440894335)
```suggestion
* Feerate diagram policy enabled in conjunction with switch to cluster mempool as of **v31.0**.
```

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2441138984)
I think the package RBF logic can have one more tweak in e6315c24326016cfaee5bd046e8b2e4e1088ac6b, which I think @sipa brought up a few weeks ago:

We currently have the requirement that the package feerate is higher than the parent feerate because "we don't want the child to be only paying anti-DoS fees" but that's more restrictive than it needs to be. It's ok if the child has a lower feerate than the parent and only pays anti-DoS fees, as long as it pays for itself to be above the mempool mi
...

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2441096106)
e6315c24326: In both single and package settings, `ImprovesFeerateDiagram` is where cluster size limits are checked for the first time if RBF is happening. The check is cached and many RBF checks are cheaper than `CheckMemPoolPolicyLimits`, so that doesn't seem problematic.

However, this is assigning `TX_RECONSIDERABLE` as the error when cluster limits are exceeded, which will cause net_processing to give it some special treatment (we may redownload and revalidate in some circumstances, which
...

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440904975)
fba5200d59af87db240a1b061f995c63b0ed5ee8

Happy for this to go into a followup:

Maybe worth adding a bit more color for people who don't have any context: A singleton replacing a singleton must pay higher total fees and have a higher feerate (covers vast majority of cases). For more complex cases, see this link to more comprehensive explanation.

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2441070793)
b9cbca76762 Reviewer note, I originally wanted to point out the extra commented line but then read the commit message and saw it changes back later in e6315c24326

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440825043)
I think the whole file can be deleted, as ancestor/descendant limits are now also obsolete.

I think we can write a new doc for cluster limits in a followup? And a release note (/me ducks)