💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2213427874)
In a0a458965c944434b1f3e4d83277257793435edb "descriptor: ToPrivateString() pass if at least 1 priv key exists"

```diff
- bool tmp{false};
- auto key_str{ctx.ToString(key, tmp)};
+ bool fragment_has_private_key{false};
+ auto key_str{ctx.ToString(key, fragment_has_private_key)};

💬 stickies-v commented on pull request "test: Do not pass tests on unhandled exceptions":
(https://github.com/bitcoin/bitcoin/pull/33001#discussion_r2213651061)
Logs with special case handling:

```
2025-07-17T15:07:39.774663Z TestFramework (ERROR): Called Process failed with 'None'
2025-07-17T15:18:22.039274Z TestFramework (WARNING): Exiting after keyboard interrupt
```

Logs without special case handling:
```
2025-07-17T15:19:09.169925Z TestFramework (ERROR): CalledProcessError(1, ['ls', '--invalid-flag'])
2025-07-17T15:19:26.639162Z TestFramework (ERROR): KeyboardInterrupt()
```

Imo, they're equally informative for KeyboardInterrupt, an
...

💬 hebasto commented on issue "Avoid plural forms in non-GUI translatable strings (lacks `%n` support)":
(https://github.com/bitcoin/bitcoin/issues/31890#issuecomment-3084488155)
> Was there any followup here?

Below is the response I received:
> Hi, Hennadii
>
> Here are some pluralization guidelines I wrote for one of my clients. Feel free to share them with the team:
>
> “Use proper plural forms
>
> Pluralization (p11n) is the process of changing nouns from the singular to the plural form. Not all languages change plural forms of nouns in the same way, some don’t have any plural form.
>
> There is a difference between plural forms and plural rules
>
> Plural form
...

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3084511037)
review ACK ec37e518682ca6c7272d1eeaea099dc9b3375e41 🌙

<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 RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK ec37e518682c
...

💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2213672519)
Has this been upstreamed / why is it needed?

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2213681709)
> Has this been upstreamed...

Not yet.

> why is it needed?

Otherwise, IWYU will suggest platform-specific headers instead of the standard one provided by the C library.

💬 mzumsande commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2213683784)
I'm skeptical if this is a possible scenario with today's index base code (which is much better with reorgs / unexpected restart scenarios than it was back then), but it's definitely more complicated than I thought, so this PR is not appropriate for this - I dropped the commit and will resolve this discussion.

💬 maflcko commented on issue "Avoid plural forms in non-GUI translatable strings (lacks `%n` support)":
(https://github.com/bitcoin/bitcoin/issues/31890#issuecomment-3084545041)
It seems a bit too much hassle to:

* Split any format string into multiple, so that transifex can handle the plural forms
* Implement runtime string-translation on plural strings (would need a manually placed macro/function call)

Seems fine to just let users figure out the plural form and then "correct the typo" while reading?

💬 hebasto commented on pull request "cmake: Create subdirectories in build tree in advance":
(https://github.com/bitcoin/bitcoin/pull/32773#issuecomment-3084555435)
Rebased to resolve conflict with merged https://github.com/bitcoin/bitcoin/pull/32881.

✅ hebasto closed an issue: "Avoid plural forms in non-GUI translatable strings (lacks `%n` support)"
(https://github.com/bitcoin/bitcoin/issues/31890)

💬 maflcko commented on pull request "ci: Run unit test parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33000#issuecomment-3084561583)
Looks like this gives a 20% faster Msan, Tsan, and previous releases task with N=1, comparing with the fastest master run in the last week:

* https://cirrus-ci.com/build/4989581210157056
* https://cirrus-ci.com/build/5602428818554880

💬 hebasto commented on pull request "ci: Enable more shellcheck":
(https://github.com/bitcoin/bitcoin/pull/32970#discussion_r2213730157)
Does `cmake -S ...` still need to be wrapped in `bash -c "..."`?

💬 maflcko commented on pull request "test: Do not pass tests on unhandled exceptions":
(https://github.com/bitcoin/bitcoin/pull/33001#discussion_r2213737305)
> more informative without special case handling for `CalledProcessError`, with less code.

I don't think this is true. It is missing the output. You can try this for any command that captures the output:

`subprocess.run(['bash', '-c', 'echo aaaaa && false'], check=True, capture_output=True)`

There could be an argument about stderr, but this seems better as a separate commit or pull.



> KeyboardInterrupt

It currently uses `self.log.warning`, which is different in verbosity. I do
...

💬 maflcko commented on pull request "ci: Enable more shellcheck":
(https://github.com/bitcoin/bitcoin/pull/32970#discussion_r2213741192)
It is not trivial to replace. Maybe the `eval` hack from below can be used:

```sh
# parses TEST_RUNNER_EXTRA as an array which allows for multiple arguments such as TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6"'

eval "TEST_RUNNER_EXTRA=($TEST_RUNNER_EXTRA)"
```

however, I haven't tried this yet.

💬 hebasto commented on pull request "ci: Enable more shellcheck":
(https://github.com/bitcoin/bitcoin/pull/32970#discussion_r2213757515)
Maybe
```suggestion
# shellcheck disable=SC2086
cmake -S "$BASE_ROOT_DIR" -B "$BASE_BUILD_DIR" $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG || (
```
?

💬 maflcko commented on pull request "ci: Enable more shellcheck":
(https://github.com/bitcoin/bitcoin/pull/32970#discussion_r2213780607)
No this won't work, because some elements contain spaces. `SC2086` is about this, to warn about word splitting.

For example. `DCMAKE_CXX_FLAGS` will be split:


```
[13:10:44.210] + cmake -S /ci_container_base -B /ci_container_base/ci/scratch/build-arm-linux-gnueabihf -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DCMAKE_TOOLCHAIN_FILE=/ci_container_base/depends/arm-linux-gnueabihf/toolchain.cmake -DWERROR=ON -DCMAKE_INSTALL_PREFIX=/ci_container_base/ci/scratch/out -Werror=dev -DREDUCE_EXPORTS=
...

💬 hebasto commented on pull request "init: [gui] Avoid UB/crash in InitAndLoadChainstate":
(https://github.com/bitcoin/bitcoin/pull/32987#issuecomment-3084665767)
Concept ACK.

👍 hebasto approved a pull request: "ci: Enable more shellcheck"
(https://github.com/bitcoin/bitcoin/pull/32970#pullrequestreview-3030212877)
ACK fa1fd074685ca96b9bd3855e9e6fe730a4f6462c.

💬 sfsegreto commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3084688654)
Thanks @maflcko , I understand this application can run over emulation, but can you help clarify a bit more what is blocking from having Windows Arm builds of this project? Is it because QT /guix building on a Linux system can't make (or distribute) a WinArm binary? Having a fully Arm build of this project running on Windows Arm would be faster and more efficient than running over the emulation.

💬 maflcko commented on pull request "ci: Enable more shellcheck":
(https://github.com/bitcoin/bitcoin/pull/32970#discussion_r2213801696)
Yeah, the eval hack should work:

```
$ export T="-DREDUCE_EXPORTS=ON -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=maybe-uninitialized'"; eval "T=($T)"; for i in "${T[@]}"; do echo "_${i}_" ; done
_-DREDUCE_EXPORTS=ON_
_-DCMAKE_CXX_FLAGS=-Wno-psabi -Wno-error=maybe-uninitialized_