💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2098614758)
It looks like `_hex_u8` can be used directly in uin256's constructor with normal byte order, so used that.

💬 mzumsande commented on pull request "test: allow all functional tests to be run or skipped with --usecli":
(https://github.com/bitcoin/bitcoin/pull/32290#discussion_r2098623556)
This is only correct for `hash_or_height`, which because of its dual use is defined as a numeric, non-string parameter and is therefore converted from json. It would be incorrect for actual string args:
You can see it directly using `bitcoin-cli` in the console:
You can do
`bitcoin-cli -regtest getblockstats '"3520a1e79bd3df078c762693b5c113469e267176aefca0d3c1dac29b8db6dd33"'`
but
`bitcoin-cli -regtest getblockstats "3520a1e79bd3df078c762693b5c113469e267176aefca0d3c1dac29b8db6dd33"` fails w
...

💬 theuni 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_r2098623686)
Ok, yes, thank you for walking through that. I agree. Pushed with a slight change (only adding to the queue if tx_ok).

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