💬 theuni commented on pull request "threading: drop CSemaphore in favor of c++20 std::counting_semaphore":
(https://github.com/bitcoin/bitcoin/pull/32466#discussion_r2098634952)
Correct. This is just for symmetry with [std::binary_semaphore](https://en.cppreference.com/w/cpp/thread/counting_semaphore)

💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.4":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2895455313)
>I think it'd be worth investigating any issues with 0.16.5, rather than deferring.

Sounds good, marking the PR as draft again while I investigate.

I've bisected to this commit in `lief`: https://github.com/lief-project/LIEF/commit/f23ced2f4ffc170d0a6f40ff4a1bee575e3447cf

Seems related to scikit-build 0.10 [changing](https://iscinumpy.dev/post/scikit-build-core-0-10/#other-changes) `cmake.targets` to `build.targets`, and defining `build.targets` explains why `build` is defined in [`conf
...

📝 davidgumberg converted_to_draft a pull request: "deps: Bump lief to 0.16.4"
(https://github.com/bitcoin/bitcoin/pull/32431)
Partially resolves https://github.com/bitcoin/bitcoin/issues/30520, updating to `lief` 0.16.4.

💬 willcl-ark commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2098640978)
Verified correct. thanks

💬 mzumsande commented on pull request "test: allow all functional tests to be run or skipped with --usecli":
(https://github.com/bitcoin/bitcoin/pull/32290#issuecomment-2895462729)
[7c13240](https://github.com/bitcoin/bitcoin/commit/7c13240d7b9a03b77ad84460e80007063367e05a) to [2bf3f90](https://github.com/bitcoin/bitcoin/commit/2bf3f907ff54efd4fa22930a81269d568e29274e): rebased - as a side effect, the legacy wallet removal made `wallet_labels.py --usecli` work on master, so no longer mentioned here.

💬 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_r2098653255)
Done

💬 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_r2098653374)
Done

💬 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_r2098653469)
Done

💬 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_r2098653553)
Done

💬 darosior commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-2895479357)
I tested this PR by rebasing #29409 on top of it and running my (updated) `core_bdk_wallet` against it.

I found that `bitcoin-node` will (still) segfault if i try to stop it while the client is running. The steps i followed were to 1) start `bitcoin-node` 2) start my PoC which will connect to it and stall with the open connection for 6 seconds 3) before the 6 seconds have elapsed, stop the `bitcoin-node` process (in this specific case by sending it a `SIGINT`). Here are the logs of the `bitco
...

💬 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_r2098654037)
`key_exp_index` doesn't really do anything for single key descriptors. I've dropped it here.

💬 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_r2098654169)
Done

💬 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