⚠️ fanquake opened an issue: "test: pycapnp doesn't support free threaded Python"
(https://github.com/bitcoin/bitcoin/issues/33582)
Python 3.14.0 has been released, and ["Free-threaded Python is officially supported"](https://docs.python.org/3/whatsnew/3.14.html#whatsnew314-free-threaded-now-supported). However using Python `3.14.0t` and running the functional tests results in a failure in `interface_ipc.py`:
```bash
./build/test/functional/test_runner.py interface_ipc.py
Temporary test directory at /var/folders/sq/z88fhjzj0b19ftsd2_bjrmjm0000gn/T/test_runner_₿_🏃_20251009_094704
Remaining jobs: [interface_ipc.py]
1/1 - inte
...
(https://github.com/bitcoin/bitcoin/issues/33582)
Python 3.14.0 has been released, and ["Free-threaded Python is officially supported"](https://docs.python.org/3/whatsnew/3.14.html#whatsnew314-free-threaded-now-supported). However using Python `3.14.0t` and running the functional tests results in a failure in `interface_ipc.py`:
```bash
./build/test/functional/test_runner.py interface_ipc.py
Temporary test directory at /var/folders/sq/z88fhjzj0b19ftsd2_bjrmjm0000gn/T/test_runner_₿_🏃_20251009_094704
Remaining jobs: [interface_ipc.py]
1/1 - inte
...
💬 fanquake commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3384824516)
cc @Sjors @ryanofsky
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3384824516)
cc @Sjors @ryanofsky
🤔 stratospher reviewed a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3317916661)
ACK 44a7261.
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3317916661)
ACK 44a7261.
💬 maflcko commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3384980563)
I'd expect there is a bunch of our own test code which relies on the GIL. I guess there is no way to find such "unsafe" Python code other than to try to run with the GIL disabled and wait for an intermittent issue to pop up at some point in time?
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3384980563)
I'd expect there is a bunch of our own test code which relies on the GIL. I guess there is no way to find such "unsafe" Python code other than to try to run with the GIL disabled and wait for an intermittent issue to pop up at some point in time?
💬 optout21 commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2416151924)
nit: This declaration of `outpoint` is enough to be placed in the `else` branch
``` diff
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index 3928120a3f..055c3c4b31 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -59,13 +59,12 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
if (random_coin.IsSpent()) {
return;
}
- COutPoin
...
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2416151924)
nit: This declaration of `outpoint` is enough to be placed in the `else` branch
``` diff
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index 3928120a3f..055c3c4b31 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -59,13 +59,12 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
if (random_coin.IsSpent()) {
return;
}
- COutPoin
...
📝 Aathish101 opened a pull request: "Update .style.yapf"
(https://github.com/bitcoin/bitcoin/pull/33583)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/33583)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 Aathish101 commented on pull request "Update .style.yapf":
(https://github.com/bitcoin/bitcoin/pull/33583#issuecomment-3384994424)
_updated_
(https://github.com/bitcoin/bitcoin/pull/33583#issuecomment-3384994424)
_updated_
💬 optout21 commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3385012657)
ACK c4293c7b21971e7ddd813ad3fbe574eb5605d60e
With the recent commit restructuring the steps are clear.
The code change is sound and looks well tested.
I've left one non-blocking nit, no other comment.
(https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3385012657)
ACK c4293c7b21971e7ddd813ad3fbe574eb5605d60e
With the recent commit restructuring the steps are clear.
The code change is sound and looks well tested.
I've left one non-blocking nit, no other comment.
💬 maflcko commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3385035667)
Generally, I'd say we should continue to treat the no-GIL as experimental, possibly accept fixes where they make sense. However, longer term, I wonder what the goal should be. According to https://peps.python.org/pep-0779/#rationale there is a performance and memory overhead of the no-GIL version. Also, the test code is largely single threaded (at least logic wise), with sync and wait loops put basically after every non-sync statement. So my guess is that, if we wanted to make meaningful use of
...
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3385035667)
Generally, I'd say we should continue to treat the no-GIL as experimental, possibly accept fixes where they make sense. However, longer term, I wonder what the goal should be. According to https://peps.python.org/pep-0779/#rationale there is a performance and memory overhead of the no-GIL version. Also, the test code is largely single threaded (at least logic wise), with sync and wait loops put basically after every non-sync statement. So my guess is that, if we wanted to make meaningful use of
...
✅ maflcko closed a pull request: "Update .style.yapf"
(https://github.com/bitcoin/bitcoin/pull/33583)
(https://github.com/bitcoin/bitcoin/pull/33583)
💬 maflcko commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2416218945)
nit: I guess a more minimal one-line temporary(?) workaround would be to just add `&` here? Ref https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3381846204
```suggestion
BOOST_CHECK_EXCEPTION(CheckNumFormatSpecifiers<WrongNumArgs>(wrong_fmt), const char*&, HasReason{error});
```
> I don't mind adding such a check, but it seemed to me contrib/devtools/bitcoin-tidy isn't actively developed.
In theory we could add a tidy check for this, but I wonder how much it is worth i
...
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2416218945)
nit: I guess a more minimal one-line temporary(?) workaround would be to just add `&` here? Ref https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3381846204
```suggestion
BOOST_CHECK_EXCEPTION(CheckNumFormatSpecifiers<WrongNumArgs>(wrong_fmt), const char*&, HasReason{error});
```
> I don't mind adding such a check, but it seemed to me contrib/devtools/bitcoin-tidy isn't actively developed.
In theory we could add a tidy check for this, but I wonder how much it is worth i
...
💬 maflcko commented on pull request "doc: bump the template macOS version since 14 is now the minimum supported version":
(https://github.com/bitcoin/bitcoin/pull/33573#discussion_r2416237758)
```suggestion
placeholder: e.g. "MacOS 26.0" or "Ubuntu 26.04 LTS"
```
nit: Those are just placeholders, so the value doesn't really matter. Though, if updating them, it may be best to update them in such a way that they won't need to be touched again for the longest time.
I understand that Ubuntu 26.04 doesn't exist yet, but there will be a daily ISO starting next month, which seems good enough.
(https://github.com/bitcoin/bitcoin/pull/33573#discussion_r2416237758)
```suggestion
placeholder: e.g. "MacOS 26.0" or "Ubuntu 26.04 LTS"
```
nit: Those are just placeholders, so the value doesn't really matter. Though, if updating them, it may be best to update them in such a way that they won't need to be touched again for the longest time.
I understand that Ubuntu 26.04 doesn't exist yet, but there will be a daily ISO starting next month, which seems good enough.
✅ maflcko closed a pull request: "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator{h,cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218)
(https://github.com/bitcoin/bitcoin/pull/33218)
💬 maflcko commented on pull request "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator{h,cpp}`":
(https://github.com/bitcoin/bitcoin/pull/33218#issuecomment-3385146910)
Re-open to trigger the newly added GHA CI tasks
(https://github.com/bitcoin/bitcoin/pull/33218#issuecomment-3385146910)
Re-open to trigger the newly added GHA CI tasks
📝 maflcko reopened a pull request: "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator{h,cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218)
This PR is a simple refactoring that does four things:
1. Renames `test/policy_fee_tests.cpp` to `test/feerounder_tests.cpp`.
2. Renames `policy/fees.{h,cpp}` to `policy/fees/block_policy_estimator.{h,cpp}`.
3. Renames `policy/fees_args.cpp` to `policy/fees/block_policy_estimator_args.cpp`.
4. Modifies `estimateSmartFee` to return the block height at which the estimate was made by adding a `best_height` unsigned int value to the `FeeCalculation` struct.
**Motivation**
In preparation
...
(https://github.com/bitcoin/bitcoin/pull/33218)
This PR is a simple refactoring that does four things:
1. Renames `test/policy_fee_tests.cpp` to `test/feerounder_tests.cpp`.
2. Renames `policy/fees.{h,cpp}` to `policy/fees/block_policy_estimator.{h,cpp}`.
3. Renames `policy/fees_args.cpp` to `policy/fees/block_policy_estimator_args.cpp`.
4. Modifies `estimateSmartFee` to return the block height at which the estimate was made by adding a `best_height` unsigned int value to the `FeeCalculation` struct.
**Motivation**
In preparation
...
👍 hebasto approved a pull request: "randomenv: Fix MinGW dllimport warning for `environ`"
(https://github.com/bitcoin/bitcoin/pull/33570#pullrequestreview-3318292404)
ACK 97762b1fcf5d389b4f9c06ae46d6408fb78627b0.
(https://github.com/bitcoin/bitcoin/pull/33570#pullrequestreview-3318292404)
ACK 97762b1fcf5d389b4f9c06ae46d6408fb78627b0.
📝 MamunC0der opened a pull request: "Upgrade GitHub Action to download-artifact@v5"
(https://github.com/bitcoin/bitcoin/pull/33584)
Release notes:https://github.com/actions/download-artifact/releases/tag/v5.0.0
Change:
uses: actions/download-artifact@v4 -> uses: actions/download-artifact@v5
(https://github.com/bitcoin/bitcoin/pull/33584)
Release notes:https://github.com/actions/download-artifact/releases/tag/v5.0.0
Change:
uses: actions/download-artifact@v4 -> uses: actions/download-artifact@v5
🤔 ismaelsadeeq reviewed a pull request: "kernel: Introduce initial C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3317819837)
Thanks for the addressing comments quickly and the thread safety clarification.
First Pass
- Overall the commits look solid. I've left a few more comments mostly non-blocking nice to haves.
This PR currently does three things, introduces the C header API, adds a C++ wrapper and tests, updates `bitcoin-chainstate` to use `libbitcoinkernel` wrapper
Maybe split this into two PRs:
- C header API, C++ wrapper here, then bitcoin-chainstate usage of the wrapper, other things can also be added befo
...
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3317819837)
Thanks for the addressing comments quickly and the thread safety clarification.
First Pass
- Overall the commits look solid. I've left a few more comments mostly non-blocking nice to haves.
This PR currently does three things, introduces the C header API, adds a C++ wrapper and tests, updates `bitcoin-chainstate` to use `libbitcoinkernel` wrapper
Maybe split this into two PRs:
- C header API, C++ wrapper here, then bitcoin-chainstate usage of the wrapper, other things can also be added befo
...
💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416004410)
In "kernel: Add functions for the block validation state to C header" 43d64194940028071b9f3774f938936d5c6b57d7
nit: this makes sense here, but won't makes sense when you are reading the logs I think, perhaps just remove the "excluding any below reasons" phrase?
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416004410)
In "kernel: Add functions for the block validation state to C header" 43d64194940028071b9f3774f938936d5c6b57d7
nit: this makes sense here, but won't makes sense when you are reading the logs I think, perhaps just remove the "excluding any below reasons" phrase?
💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416094811)
In "kernel: Add functions to read block from disk to C header" 8653972e2b0c3c698c4fc426ebfd23d44be14983
I think it will be worth indicating the several scenarios that the user should guard here not just a single example.
or provide a one fit all example of how that inconsistency can be prevented.
```suggestion
* chainstate manager, e.g. processing blocks, will change the chain.
```
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416094811)
In "kernel: Add functions to read block from disk to C header" 8653972e2b0c3c698c4fc426ebfd23d44be14983
I think it will be worth indicating the several scenarios that the user should guard here not just a single example.
or provide a one fit all example of how that inconsistency can be prevented.
```suggestion
* chainstate manager, e.g. processing blocks, will change the chain.
```