π¬ purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199808754)
> FWIW, the same effect can be achieved with the `--no-wrap` option. However, I havenβt tested this with older versions of `xgettext`.
I have tested it. It simplifies the loop a lot.
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199808754)
> FWIW, the same effect can be achieved with the `--no-wrap` option. However, I havenβt tested this with older versions of `xgettext`.
I have tested it. It simplifies the loop a lot.
π¬ Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3199834585)
It seems @achow101 (https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3129705803) and @fanquake (https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3179426824) disagree on the question whether to have IPC enable in the source build (non dev preset). I don't have a strong preference.
Capnp is substantially quicker to install than qt and doesn't slow down the full build as much as the GUI does, so that's not the best comparison. Comparison to the wallet isn't perfect eithe
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3199834585)
It seems @achow101 (https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3129705803) and @fanquake (https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3179426824) disagree on the question whether to have IPC enable in the source build (non dev preset). I don't have a strong preference.
Capnp is substantially quicker to install than qt and doesn't slow down the full build as much as the GUI does, so that's not the best comparison. Comparison to the wallet isn't perfect eithe
...
π hebasto approved a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131382964)
re-ACK 3c4a109aa821cbf1e46a67275b4f456673ed13d8.
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131382964)
re-ACK 3c4a109aa821cbf1e46a67275b4f456673ed13d8.
π¬ purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199861216)
> Apparently, its behaviour is a bit complicated.
For reference, the problem is that strings that are wrapped in C++ are not joined with `--no-wrap`.
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199861216)
> Apparently, its behaviour is a bit complicated.
For reference, the problem is that strings that are wrapped in C++ are not joined with `--no-wrap`.
π¬ Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284635042)
As of cd86ddaa1dac104415ab16cc8068fd50d068c12f the test passes for me on macOS with Python 3.10.14 with pycapnp installed via pip and capnp installed via homebrew.
I also tested Python 3.12.11. With Python 3.13.17 `pip3 install pycapnp` fails. We can worry about that later.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284635042)
As of cd86ddaa1dac104415ab16cc8068fd50d068c12f the test passes for me on macOS with Python 3.10.14 with pycapnp installed via pip and capnp installed via homebrew.
I also tested Python 3.12.11. With Python 3.13.17 `pip3 install pycapnp` fails. We can worry about that later.
π¬ Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3199936994)
If this PR is to be based on #31802 or #33190 then it should be marked draft. Otherwise it could be made independent by dropping 7a7f11933b83d290129377523cc4a3cfc45fe9ef and instead enabling IPC for one or more CI machines that run functional tests. It should be straightforward to rebase #31802 as long as you leave `ci/test/00_setup_env_i686_multiprocess.sh` alone (due to 789f7195b2acbc3e50ee75b2b5a88af058cf7b14). My preference would be to base it on #31802 because that gives the widest test cov
...
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3199936994)
If this PR is to be based on #31802 or #33190 then it should be marked draft. Otherwise it could be made independent by dropping 7a7f11933b83d290129377523cc4a3cfc45fe9ef and instead enabling IPC for one or more CI machines that run functional tests. It should be straightforward to rebase #31802 as long as you leave `ci/test/00_setup_env_i686_multiprocess.sh` alone (due to 789f7195b2acbc3e50ee75b2b5a88af058cf7b14). My preference would be to base it on #31802 because that gives the widest test cov
...
π¬ maflcko commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284658673)
from the llm:
```suggestion
git clone https://github.com/capnproto/pycapnp.git
```
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284658673)
from the llm:
```suggestion
git clone https://github.com/capnproto/pycapnp.git
```
π¬ maflcko commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284671990)
Not sure why the lint task complains here:
```
[21:49:19.712] test/functional/test_framework/test_framework.py:560: error: Need type annotation for "extra_init" [var-annotated]
[21:49:21.445] Found 1 error in 1 file (checked 301 source files)
[21:49:21.473] ^---- β οΈ Failure generated from lint-python.py
```
No other variable in this function is required to be annotated and the lint should probably be fixed to not require it here either.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284671990)
Not sure why the lint task complains here:
```
[21:49:19.712] test/functional/test_framework/test_framework.py:560: error: Need type annotation for "extra_init" [var-annotated]
[21:49:21.445] Found 1 error in 1 file (checked 301 source files)
[21:49:21.473] ^---- β οΈ Failure generated from lint-python.py
```
No other variable in this function is required to be annotated and the lint should probably be fixed to not require it here either.
π¬ Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284683975)
In cd86ddaa1dac104415ab16cc8068fd50d068c12f _ci: add functional test for IPC interface_: is it somehow possible for `import` to succeed but capnp to remain undefined?
https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3197934560
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284683975)
In cd86ddaa1dac104415ab16cc8068fd50d068c12f _ci: add functional test for IPC interface_: is it somehow possible for `import` to succeed but capnp to remain undefined?
https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3197934560
π¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284714840)
Was concerned about increasing the runtime of the tests due to repeatedly re-generating `first_chain`/`second_chain` as documented in the commit message:
> The approach to use one BOOST_AUTO_TEST_CASE for each scenario was attempted but the common initialization is repeated for every test case using the same fixture. This made the local runtime go from 0.39s to 0.65s. Would rather avoid global fixtures, so continued with one boost test case for now.
However, I figured out an okay way to swit
...
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284714840)
Was concerned about increasing the runtime of the tests due to repeatedly re-generating `first_chain`/`second_chain` as documented in the commit message:
> The approach to use one BOOST_AUTO_TEST_CASE for each scenario was attempted but the common initialization is repeated for every test case using the same fixture. This made the local runtime go from 0.39s to 0.65s. Would rather avoid global fixtures, so continued with one boost test case for now.
However, I figured out an okay way to swit
...
π€ BrandonOdiwuor reviewed a pull request: "build: Enable ENABLE_IPC option by default"
(https://github.com/bitcoin/bitcoin/pull/33190#pullrequestreview-3131561164)
Verified that `ENABLE_IPC` is enabled by default and that both `bitcoin-node` and `bitcoin-gui` (when built with -DBUILD_GUI=ON) are included in `build/bin`.
```bash
$ cmake -B build -DENABLE_GUI=ON
```
(system: macOS 15.6)
<img width="1512" height="912" alt="Screenshot 2025-08-19 at 11 32 05" src="https://github.com/user-attachments/assets/2bd88841-6e0b-4054-bff0-f6b224a1cc9e" />
(https://github.com/bitcoin/bitcoin/pull/33190#pullrequestreview-3131561164)
Verified that `ENABLE_IPC` is enabled by default and that both `bitcoin-node` and `bitcoin-gui` (when built with -DBUILD_GUI=ON) are included in `build/bin`.
```bash
$ cmake -B build -DENABLE_GUI=ON
```
(system: macOS 15.6)
<img width="1512" height="912" alt="Screenshot 2025-08-19 at 11 32 05" src="https://github.com/user-attachments/assets/2bd88841-6e0b-4054-bff0-f6b224a1cc9e" />
π€ BrandonOdiwuor reviewed a pull request: "build: Enable ENABLE_IPC option by default"
(https://github.com/bitcoin/bitcoin/pull/33190#pullrequestreview-3131564457)
I still don't understand the value of enabling these binaries in the build tree without also including them in release artifact compared to https://github.com/bitcoin/bitcoin/pull/31802
(https://github.com/bitcoin/bitcoin/pull/33190#pullrequestreview-3131564457)
I still don't understand the value of enabling these binaries in the build tree without also including them in release artifact compared to https://github.com/bitcoin/bitcoin/pull/31802
β
glozow closed an issue: "Add support for creating v3 raw transactions in `createrawtransaction` RPC"
(https://github.com/bitcoin/bitcoin/issues/31348)
(https://github.com/bitcoin/bitcoin/issues/31348)
π glozow merged a pull request: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896)
(https://github.com/bitcoin/bitcoin/pull/32896)
π¬ BrandonOdiwuor commented on pull request "doc: gen-manpages.py should check build options":
(https://github.com/bitcoin/bitcoin/pull/33085#issuecomment-3200113920)
@janb84 thanks for having a look
I think introducing a main function would require a huge refactor to the file for such small check.
I would also love to hear some advantages of adding a main function here
(https://github.com/bitcoin/bitcoin/pull/33085#issuecomment-3200113920)
@janb84 thanks for having a look
I think introducing a main function would require a huge refactor to the file for such small check.
I would also love to hear some advantages of adding a main function here
π¬ ismaelsadeeq commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3200160757)
Nice idea.
A couple of questions:
1. How do nodes build the block template they share with peers? When they use only their mempool to build those templates, I donβt think that the transactions with diverging mempool policy will relay through the network and improve the compact block reconstruction process via this mechanism. Only peers whose outbound connections are miners will benefit.
2. We donβt accept transactions in the mempool that have divergent policy because we deem them non-st
...
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3200160757)
Nice idea.
A couple of questions:
1. How do nodes build the block template they share with peers? When they use only their mempool to build those templates, I donβt think that the transactions with diverging mempool policy will relay through the network and improve the compact block reconstruction process via this mechanism. Only peers whose outbound connections are miners will benefit.
2. We donβt accept transactions in the mempool that have divergent policy because we deem them non-st
...
π hebasto opened a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin-core/gui/pull/884)
This change avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
(https://github.com/bitcoin-core/gui/pull/884)
This change avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
π¬ sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284831733)
This was with an earlier version of the PR which called `load_capnp_modules` from `set_test_params` (which tried to do so before deciding whether pycapnp was even installed).
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284831733)
This was with an earlier version of the PR which called `load_capnp_modules` from `set_test_params` (which tried to do so before deciding whether pycapnp was even installed).
π€ hodlinator reviewed a pull request: "headerssync: Preempt unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3131691825)
Latest push:
* Breaks up test into multiple boost test cases instead of regular functions (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2277678459). In doing so, I changed back to using a custom fixture as on master. Diffing with dimmed-zebra helps show what was just moved and what actually changed `git diff 342359f 53341ea`.
* Removed redundant `assert()` after bringing that block closer to constants (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2271294627).
* Minor
...
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3131691825)
Latest push:
* Breaks up test into multiple boost test cases instead of regular functions (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2277678459). In doing so, I changed back to using a custom fixture as on master. Diffing with dimmed-zebra helps show what was just moved and what actually changed `git diff 342359f 53341ea`.
* Removed redundant `assert()` after bringing that block closer to constants (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2271294627).
* Minor
...
π¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284849835)
Pushed version with multiple test cases.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284849835)
Pushed version with multiple test cases.
π¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284838171)
Moved up the macro above the constants in the latest push, and brought the assert block up closer to the constants - in the process removing the final assert.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284838171)
Moved up the macro above the constants in the latest push, and brought the assert block up closer to the constants - in the process removing the final assert.