💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2098654240)
Done

💬 achow101 commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#issuecomment-2895483233)
ACK 135a0f0aa711b95c50aa4cbe0c38d82d647f1c8b

💬 davidgumberg commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2098663304)
Although, the phrase "size specifiers" is used here which I believe is incorrect, the spirit of this remark is still helpful.

Using the terminology from this article: https://en.cppreference.com/w/c/io/fprintf I believe the spirit of this note is, "- For `strprintf`, `LogInfo`, `LogDebug`, etc: When using conversion specifiers like `%d`, don't use length modifiers (e.g.: `%ld`).

💬 davidgumberg commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2098669764)
I slightly disagree with removing this: I personally found this note helpful pre cmake migration, when the recommended way to generate `compile_commands.json` was with `bear`, and Bitcoin Core's `.gitignore` didn't exclude this.

🚀 achow101 merged a pull request: "doc: Add missing top-level description to pruneblockchain RPC"
(https://github.com/bitcoin/bitcoin/pull/32333)

💬 maflcko commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2098673886)
I see, but I don't think it matters. If it mattered, we should remove the existing length modifiers in the code and enforce this rule at compile time (`ConstevalFormatString`). If it doesn't matter, there is no need to document it.

💬 maflcko commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2098673935)
This was added to the gitignore file itself in commit
fada115cbeaa8c0ca3d19178499079b66ee5f499. The past violations are
evidence that the note in the dev notes was missed anyway.

I've listed some in https://github.com/bitcoin/bitcoin/pull/32417#issue-3038116748

If there is a file-specific note, it seems easier to just have it in the file itself.

💬 achow101 commented on pull request "threading: drop CSemaphore in favor of c++20 std::counting_semaphore":
(https://github.com/bitcoin/bitcoin/pull/32466#issuecomment-2895509210)
ACK 6f7052a7b96f058568af9aed2f014ae7a25e0f68

💬 achow101 commented on pull request "Wallet: Fix Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor":
(https://github.com/bitcoin/bitcoin/pull/32344#issuecomment-2895537874)
ACK 97d383af6d54b463da64f45680a146d7e93eb146

🚀 achow101 merged a pull request: "threading: drop CSemaphore in favor of c++20 std::counting_semaphore"
(https://github.com/bitcoin/bitcoin/pull/32466)

✅ achow101 closed an issue: "Bug: Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in `AddWalletDescriptor`"
(https://github.com/bitcoin/bitcoin/issues/31728)

🚀 achow101 merged a pull request: "Wallet: Fix Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor"
(https://github.com/bitcoin/bitcoin/pull/32344)

💬 davidgumberg commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2895646341)
> > This change only partially resolves #32391
>
> What is left unsolved after this PR?

This comment from above: https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2846972614

Points out that the deprecated Windows `Crypt*` functions are still [invoked](https://github.com/gcc-mirror/gcc/blob/fba34a0cc55488ad89becf81cf2c9ac517d244d4/libssp/ssp.c#L80-L92) by GCC's `libssp` (`-fstack-protector`).

So the bitcoin core binaries won't really be free of those functions until GCC merge
...

📝 maflcko opened a pull request: "ci: Avoid && dropping errors"
(https://github.com/bitcoin/bitcoin/pull/32573)
In bash, `&&` will ignore errexit. This can lead to silently ignoring errors. Compare the output of:

```
$ bash -c 'set -xe; false && false ; true; echo $?'
+ false
+ true
+ echo 0
0
```

In theory this could be fixed by using a subshell:

```
$ bash -c 'set -xe; ( false && false ) ; true; echo $?'
+ false
```

However, it is easier to just remove the `&&`.

This was introduced in commit faa807bdf8c3002a28005b4765604f518a6f2736

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2098753436)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091447427

> I would like if it will emit the help text of `rpc` command instead.

Makes sense! I expanded the "Integrating help" followup note in the description and linked to this comment.

💬 willcl-ark commented on pull request "depends: use "mkdir -p" when installing xproto":
(https://github.com/bitcoin/bitcoin/pull/32568#issuecomment-2895686124)
Mismatching hashes on my build:

```
src/core/bitcoin on  alpine_MKDIRPROG:refs/pull/32568/head [$] via △ v3.31.6 via 🐍 v3.12.10 via ❄️ impure (nix-shell-env) took 1h13m42s
❯ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum

cb7dbe6112cdd38db9757af64cd4e617cdced8a6525db8a1469f3f35d29de9f6 guix-build-df9ebbf659d5/output/aarch64-linux-gnu/SHA256SUMS.part
a0b2b6c3fd30ae9aa7667b99f0ac2fe7d59725fff6fef547bd73f2cc5b50b1b3
...

⚠️ hebasto opened an issue: "`system_tests/run_command` test fails on illumos OS"
(https://github.com/bitcoin/bitcoin/issues/32574)
On OpenIndiana:
```
./test/system_tests.cpp(54): error: in "system_tests/run_command": check what.find(tfm::format("RunCommandParseJSON error: process(%s) returned", command)) != std::string::npos has failed
./test/system_tests.cpp(55): error: in "system_tests/run_command": check what.find(expected) != std::string::npos has failed
```

Same on [OmniOS](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/15121791773/job/42505618501):
```
./test/system_tests.cpp(54): error: in "system_tes
...

💬 maflcko commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2895744563)
Looks like CI failed

💬 maflcko commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2895764479)
> users can use `createrawtransaction` to create a v3 transaction, fund it with `fundrawtransaction`, and then wind up with a v3 transaction that they cannot broadcast.

`createrawtransaction` doesn't do other policy checks either, such as OP_RETURN checks, so one could argue this is fine as well.

Though, CI is failing on this pull.

💬 maflcko commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#discussion_r2098827990)
```suggestion
"Returns the messages count and total number of bytes for network traffic.\n"
```

the ci failure is a bit vague, but I guess this would fix it