💬 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

💬 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
...