💬 andrewtoth commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2440729584)
Yes, I think we can now remove the conditions `!stack.back()->map().contains(op) && !coin.IsSpent()`.
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2440729584)
Yes, I think we can now remove the conditions `!stack.back()->map().contains(op) && !coin.IsSpent()`.
💬 snakamoto129-dev commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3416509771)
El vie, 17 oct 2025, 19:20, Pieter Wuille ***@***.***>
escribió:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/net_processing.cpp
> <https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2440648496>:
>
> > + // Determine the forking point between the peer's chain and our chain:
> + // pindexLastCommonBlock is required to be an ancestor of pindexBestKnownBlock, and will be used as a starting point.
> + // It is being set to the
...
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3416509771)
El vie, 17 oct 2025, 19:20, Pieter Wuille ***@***.***>
escribió:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/net_processing.cpp
> <https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2440648496>:
>
> > + // Determine the forking point between the peer's chain and our chain:
> + // pindexLastCommonBlock is required to be an ancestor of pindexBestKnownBlock, and will be used as a starting point.
> + // It is being set to the
...
💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2433879534)
nit: extra commented line
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2433879534)
nit: extra commented line
💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2436027616)
My bad on the linter, lgtm!
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2436027616)
My bad on the linter, lgtm!
💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2437193014)
I ended up writing some feerate diagram tests to check my understanding of how it works. It might be helpful for reviewers and/or if you're interested in taking them:
```diff
diff --git a/test/functional/mempool_cluster.py b/test/functional/mempool_cluster.py
index ee8005a1cd2..c86e6a1b5c0 100755
--- a/test/functional/mempool_cluster.py
+++ b/test/functional/mempool_cluster.py
@@ -7,6 +7,8 @@
from decimal import Decimal
from test_framework.mempool_util import (
+ assert_equal_
...
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2437193014)
I ended up writing some feerate diagram tests to check my understanding of how it works. It might be helpful for reviewers and/or if you're interested in taking them:
```diff
diff --git a/test/functional/mempool_cluster.py b/test/functional/mempool_cluster.py
index ee8005a1cd2..c86e6a1b5c0 100755
--- a/test/functional/mempool_cluster.py
+++ b/test/functional/mempool_cluster.py
@@ -7,6 +7,8 @@
from decimal import Decimal
from test_framework.mempool_util import (
+ assert_equal_
...
💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2436684815)
This started failing when I added it to other tests, then I realized it's the wrong direction
```suggestion
assert_greater_than_or_equal(last_val[1] * x['vsize'], last_val[0] * x['fee'])
```
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2436684815)
This started failing when I added it to other tests, then I realized it's the wrong direction
```suggestion
assert_greater_than_or_equal(last_val[1] * x['vsize'], last_val[0] * x['fee'])
```
🤔 sipa reviewed a pull request: "p2p: Advance pindexLastCommonBlock early after connecting blocks"
(https://github.com/bitcoin/bitcoin/pull/32180#pullrequestreview-3351389434)
utACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
(https://github.com/bitcoin/bitcoin/pull/32180#pullrequestreview-3351389434)
utACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
📝 Raimo33 opened a pull request: "refactor: optimize: reuse containers in transaction policy verification"
(https://github.com/bitcoin/bitcoin/pull/33645)
## Summary
Currently, some methods in `policy.cpp` are inefficiently reallocating containers on each loop iteration.
This PR aims at optimizing policy verifications (transactions sanitization) by reducing redundant heap allocations without losing performance even in worst case scenarios.
These improvements are particularly beneficial during validation, mostly in IBD.
## Details
I moved `std::vector` declarations outside the loop to reduce heap allocations.
This change allows the
...
(https://github.com/bitcoin/bitcoin/pull/33645)
## Summary
Currently, some methods in `policy.cpp` are inefficiently reallocating containers on each loop iteration.
This PR aims at optimizing policy verifications (transactions sanitization) by reducing redundant heap allocations without losing performance even in worst case scenarios.
These improvements are particularly beneficial during validation, mostly in IBD.
## Details
I moved `std::vector` declarations outside the loop to reduce heap allocations.
This change allows the
...
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2440919133)
Yes. Up to block 381999 on mainnet.
Parallel run, ~20 minutes:
2025-10-16T13:58:05Z initload thread exit
2025-10-16T13:58:35Z Syncing txindex with block chain from height 189999
2025-10-16T13:59:12Z Syncing txindex with block chain from height 211999
2025-10-16T13:59:42Z Syncing txindex with block chain from height 223999
2025-10-16T14:00:13Z Syncing txindex with block chain from height 235999
....
2025-10-16T14:16:01Z Syncing txindex with block chain from height 373999
2025-10-16T1
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2440919133)
Yes. Up to block 381999 on mainnet.
Parallel run, ~20 minutes:
2025-10-16T13:58:05Z initload thread exit
2025-10-16T13:58:35Z Syncing txindex with block chain from height 189999
2025-10-16T13:59:12Z Syncing txindex with block chain from height 211999
2025-10-16T13:59:42Z Syncing txindex with block chain from height 223999
2025-10-16T14:00:13Z Syncing txindex with block chain from height 235999
....
2025-10-16T14:16:01Z Syncing txindex with block chain from height 373999
2025-10-16T1
...
📝 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
...
(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)
(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
...
(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
...
(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.
(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)
(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?
(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?
(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
...
(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.
(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.
(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.