💬 fqlx commented on pull request "rpc: require integer verbosity; remove boolean 'verbose'":
(https://github.com/bitcoin/bitcoin/pull/33213#issuecomment-3198968576)
@DrahtBot Can you reopen?
(https://github.com/bitcoin/bitcoin/pull/33213#issuecomment-3198968576)
@DrahtBot Can you reopen?
💬 fqlx commented on pull request "rpc: require integer verbosity; remove boolean 'verbose'":
(https://github.com/bitcoin/bitcoin/pull/33213#issuecomment-3198997112)
Hi @fanquake @glozow @achow101 @hebasto — DrahtBot auto-closed this, but I’d like to request a reopen.
This PR removes legacy boolean verbosity and standardizes integer verbosity across RPCs (deprecated since 2017), updates docs/tests, and includes release notes. Happy to split if needed. Thanks!
(https://github.com/bitcoin/bitcoin/pull/33213#issuecomment-3198997112)
Hi @fanquake @glozow @achow101 @hebasto — DrahtBot auto-closed this, but I’d like to request a reopen.
This PR removes legacy boolean verbosity and standardizes integer verbosity across RPCs (deprecated since 2017), updates docs/tests, and includes release notes. Happy to split if needed. Thanks!
📝 fqlx opened a pull request: "Rpc no bool verbosity"
(https://github.com/bitcoin/bitcoin/pull/33214)
### Summary
Standardize RPC verbosity to integers only and remove boolean handling for clarity, consistency, and future extensibility.
### Rationale
- **Legacy cleanup**: Boolean verbosity has been discouraged/deprecated in docs since 2017 and continues to create tech debt (special-case parsing, inconsistent tests, user confusion).
- **Consistency**: Integers enable multi-level output without overloading a boolean.
### User-visible changes
- getblock, getrawtransaction, getrawmemp
...
(https://github.com/bitcoin/bitcoin/pull/33214)
### Summary
Standardize RPC verbosity to integers only and remove boolean handling for clarity, consistency, and future extensibility.
### Rationale
- **Legacy cleanup**: Boolean verbosity has been discouraged/deprecated in docs since 2017 and continues to create tech debt (special-case parsing, inconsistent tests, user confusion).
- **Consistency**: Integers enable multi-level output without overloading a boolean.
### User-visible changes
- getblock, getrawtransaction, getrawmemp
...
🤔 hebasto reviewed a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131093710)
I have reviewed d395abac85a2acf0717859f05ccfccf8a3c71a46.
> Changes:
>
> * xgettext is instructed to sort the output by file
>
> * the messages are no longer sorted by cmake
Could we revert these changes? All strings in `src/qt/bitcoinstrings.cpp` sahre the same context, namely "bitcoin-core". Keeping the content stable by sorting within the context feels more natural. Otherwise, the string order depends on the order in which source files are processed by `xgettext`.
> *
...
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131093710)
I have reviewed d395abac85a2acf0717859f05ccfccf8a3c71a46.
> Changes:
>
> * xgettext is instructed to sort the output by file
>
> * the messages are no longer sorted by cmake
Could we revert these changes? All strings in `src/qt/bitcoinstrings.cpp` sahre the same context, namely "bitcoin-core". Keeping the content stable by sorting within the context feels more natural. Otherwise, the string order depends on the order in which source files are processed by `xgettext`.
> *
...
💬 hebasto commented on pull request "Release: Prepare "Open Transifex translations for v30.0" step":
(https://github.com/bitcoin/bitcoin/pull/33152#issuecomment-3199595855)
@laanwj
> Last commit reproduces also on Ubuntu 24.04, with only harmless differences in C string splitting like
>
> ```
> "Outbound connections restricted to CJDNS (-onlynet=cjdns) but "
> "-cjdnsreachable is not provided"),
...
(https://github.com/bitcoin/bitcoin/pull/33152#issuecomment-3199595855)
@laanwj
> Last commit reproduces also on Ubuntu 24.04, with only harmless differences in C string splitting like
>
> ```
> "Outbound connections restricted to CJDNS (-onlynet=cjdns) but "
> "-cjdnsreachable is not provided"),
...
💬 hebasto commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199647537)
@purpleKarrot
> Translate the `share/qt/extract_strings_qt.py` script to CMake. This removes the python dependency from the `translate` target.
Mind updating the PR description with something like "Resolves #33146"?
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199647537)
@purpleKarrot
> Translate the `share/qt/extract_strings_qt.py` script to CMake. This removes the python dependency from the `translate` target.
Mind updating the PR description with something like "Resolves #33146"?
💬 purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199648782)
> Otherwise, the string order depends on the order in which source files are processed by `xgettext`.
@hebasto, I just created two files `a.c` and `b.c` with some `gettext` content and compared the outputs of `xgettext a.c b.c` and `xgettext b.c a.c` both with `--sort-by-file` and without. I can confirm that without the `--sort-by-file` option the order of processing changes the output, but it is stable when that option is used.
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199648782)
> Otherwise, the string order depends on the order in which source files are processed by `xgettext`.
@hebasto, I just created two files `a.c` and `b.c` with some `gettext` content and compared the outputs of `xgettext a.c b.c` and `xgettext b.c a.c` both with `--sort-by-file` and without. I can confirm that without the `--sort-by-file` option the order of processing changes the output, but it is stable when that option is used.
💬 hebasto commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199693425)
> > Otherwise, the string order depends on the order in which source files are processed by `xgettext`.
>
> @hebasto, I just created two files `a.c` and `b.c` with some `gettext` content and compared the outputs of `xgettext a.c b.c` and `xgettext b.c a.c` both with `--sort-by-file` and without. I can confirm that without the `--sort-by-file` option the order of processing changes the output, but it is stable when that option is used.
Right my assumption was not correct.
However, there
...
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199693425)
> > Otherwise, the string order depends on the order in which source files are processed by `xgettext`.
>
> @hebasto, I just created two files `a.c` and `b.c` with some `gettext` content and compared the outputs of `xgettext a.c b.c` and `xgettext b.c a.c` both with `--sort-by-file` and without. I can confirm that without the `--sort-by-file` option the order of processing changes the output, but it is stable when that option is used.
Right my assumption was not correct.
However, there
...
👍 hebasto approved a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131218780)
ACK 3c4a109aa821cbf1e46a67275b4f456673ed13d8.
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131218780)
ACK 3c4a109aa821cbf1e46a67275b4f456673ed13d8.
💬 hebasto commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199745615)
> * multiline strings are joined
FWIW, the same effect can be achieved with the `--no-wrap` option. However, I haven’t tested this with older versions of `xgettext`.
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3199745615)
> * multiline strings are joined
FWIW, the same effect can be achieved with the `--no-wrap` option. However, I haven’t tested this with older versions of `xgettext`.
💬 maflcko commented on issue "ci: failure in `logging_tests`":
(https://github.com/bitcoin/bitcoin/issues/33195#issuecomment-3199748767)
> It's still a bit surprising that this test suddenly took so long to run, but I suppose with hardware being shared these kinds of occurrences need to be accounted for?
Yeah, it seems odd that there is an intermittent 20x slowdown. It could be a hardware fault, and hopefully new CI hardware will be deployed soon. However, we also want the test to pass locally (in the rare case when someone uses a slow USB thumb drive) or on cloud boxes (which sometimes use slow network storage).
> Interesting.
...
(https://github.com/bitcoin/bitcoin/issues/33195#issuecomment-3199748767)
> It's still a bit surprising that this test suddenly took so long to run, but I suppose with hardware being shared these kinds of occurrences need to be accounted for?
Yeah, it seems odd that there is an intermittent 20x slowdown. It could be a hardware fault, and hopefully new CI hardware will be deployed soon. However, we also want the test to pass locally (in the rare case when someone uses a slow USB thumb drive) or on cloud boxes (which sometimes use slow network storage).
> Interesting.
...
💬 maflcko commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3199775857)
Thx. This is a nice and simple change to make the test log smaller and thus easier to debug in case of failure. Also, increasing the window should be harmless and could avoid intermittent issues on slow IO.
review ACK ab5c3be4b384d48d514b06dc8a391f1efa51ea38 🌤
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhv
...
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3199775857)
Thx. This is a nice and simple change to make the test log smaller and thus easier to debug in case of failure. Also, increasing the window should be harmless and could avoid intermittent issues on slow IO.
review ACK ab5c3be4b384d48d514b06dc8a391f1efa51ea38 🌤
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhv
...
💬 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.