💬 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

💬 fjahr commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2098836650)
This whole conversation reminded me that I really wanted to split up `CheckInputScripts` after interacting with it for the batch validation stuff. I will open a draft of that for conceptual feedback.

💬 fjahr commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#issuecomment-2895776416)
re-ACK fd290730f530a8b76a9607392f49830697cdd7c5

💬 maflcko commented on pull request "Prune locks":
(https://github.com/bitcoin/bitcoin/pull/19463#discussion_r2098837042)
```suggestion
"Returns a list of pruning locks.\n",
```

the ci failure is a bit vague, but this should fix it