💬 maflcko commented on pull request "doc: miscellaneous changes":
(https://github.com/bitcoin/bitcoin/pull/32644#issuecomment-2933966649)
review ACK e50312eab0b54f338d6e08bea563c352dc2de1db 🥗

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK e50312eab0b5
...

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2933974799)
> @bigspider OS (firmware) version 2.4.2. I tried removing and reinstalling Bitcoin Test, but that didn't bump the version. Will await your update. So you're testing on mainnet then? :-)

@Sjors thanks for pointing that out, you should now be able to find version 2.4.0 in the store!

💬 maflcko commented on issue "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client":
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2933976634)
My recommendation would be to do a rescan. In theory you could recover funds (not the history) from the utxo set, but this can be incomplete and fragile.

💬 hebasto commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2933979501)
> Guix build is broken:

```diff
--- a/contrib/guix/manifest.scm
+++ b/contrib/guix/manifest.scm
@@ -173,6 +173,7 @@ chain for " target " development."))
(patches (search-our-patches "lief-scikit-0-9.patch"))))
(build-system pyproject-build-system)
(native-inputs (list cmake-minimal
+ ninja
python-scikit-build-core
python-pydantic-core
python-pydantic-2))
``
...

💬 fanquake commented on pull request "refactor: Drop unused `#include <boost/operators.hpp>`":
(https://github.com/bitcoin/bitcoin/pull/32668#issuecomment-2934007819)
> There are still some places where it is incorrectly suggested, but this seems unrelated:

It seems related enough, especially if we are going to do #31308. This PR is pointing at the IWYU output to remove this include from 1 file, and doing so, but then ignoring the IWYU output saying to add it to multiple others? Seems like this could wait, or atleast be bundled with a change that adds a mapping file or similar, that makes the IWYU output make sense, otherwise how are devs meant to know whe
...

📝 hebasto converted_to_draft a pull request: "refactor: Drop unused `#include <boost/operators.hpp>`"
(https://github.com/bitcoin/bitcoin/pull/32668)
From https://api.cirrus-ci.com/v1/task/5082537556443136/logs/ci.log:
```
[10:04:20.418] /ci_container_base/src/node/mini_miner.cpp should remove these lines:
[10:04:20.418] - #include <boost/operators.hpp> // lines 8-8
```

💬 i-am-yuvi commented on pull request "[WIP] test: Fix reorg patterns in mempool tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2123054841)
ahh okay!

💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#issuecomment-2934023355)
Hi @achow101 and @maflcko I've fixed your review comments.

💬 davidgumberg commented on pull request "thread-safety: fix annotations with REVERSE_LOCK":
(https://github.com/bitcoin/bitcoin/pull/32465#issuecomment-2934027032)
Concept ACK on dropping non-RAII locks/unlocks. Maybe this can be done without reverse locks, e.g. a (maybe dumb) approach at replacing REVERSE_LOCK with LOCK*'s: https://github.com/davidgumberg/bitcoin/commit/fde4e277550095ade852ebbe09bf2f0c7533a42e

👍 dergoegge approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2891200296)
ACK a189d636184b1c28fa4a325b56c1fab8f44527b1

👍 hebasto approved a pull request: "depends: Bump boost to 1.88.0 and use new CMake buildsystem"
(https://github.com/bitcoin/bitcoin/pull/32665#pullrequestreview-2891203225)
ACK 3a350c8a1b51e140e2689e10dca338470e8deef2.

💬 l3x3l commented on issue "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client":
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2934051046)
@maflcko How do i do the utxo?

Also @achow101 I checked the best block from the dump I got and it gives me a more recent version:
Best block (modern): dummy (version)=210201, block

I also tried searching for transactions using the hashes in the dump and putting them in the blockchain explorer, but i didn't get any results.

💬 i-am-yuvi commented on pull request "test: Fix reorg patterns in mempool tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2123080418)
got it!

🚀 fanquake merged a pull request: "doc: miscellaneous changes"
(https://github.com/bitcoin/bitcoin/pull/32644)

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2934077835)
@bigspider got it! Registration seems to work and device recognized which of the keys is "ours". Though after approval async-hwi threw `Error: Device("ClientError(\n \"Failed to parse descriptor\",\n)")` and did not return an HMAC.


💬 davidgumberg commented on pull request "build: add -Wthread-safety-pointer":
(https://github.com/bitcoin/bitcoin/pull/32647#issuecomment-2934095252)
Tested ACK https://github.com/bitcoin/bitcoin/pull/32647/commits/83bfe1485c37d407de7eed11b8ad769a05f78b66

Built with recent master clang (`clang version 21.0.0git (git@github.com:llvm/llvm-project.git 88c1403981dee9844042a99dc357d8034cf5d197`)

<details>

<summary> CMake Configure Summary </summary>

```console
$ CC=clang CXX=clang++ cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake
Configure summary
=================
Executables:
bitcoin ..................
...

💬 maflcko commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2123113095)
> @maflcko Nope. I only used chatgpt to study the functions. However, every letter in the PR is typed by me. :)

I think it is clear that you didn't type the 53 thousand character long magic string yourself. It is synthetically generated via an LLM or otherwise and impossible to review.

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2934119709)
@Sjors async-hwi probably doesn't support `musig()` in descriptors. Rust libraries are generally waiting for upstream support in rust-bitcoin.

The python package [ledger-bitcoin](https://pypi.org/project/ledger-bitcoin/) is currently the only client library that is expected to work with musig2 (but it doesn't have a CLI).

💬 davidgumberg commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2123141115)
If I'm not mistaken, this `if` branch won't be reached if the line above fails.

💬 dergoegge commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-2934186705)
I vaguely remember having trouble with the unit tests on windows too. Iirc it had something to do with windows line endings being different (i.e. `\r\n` on windows vs `\n` on linux), which throws off the size accounting.