💬 diegoviola commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3574162074)
`dlg->activateWindow()` is essentially the same as calling `GUIUtil::bringToFront(dlg)`.

💬 hodlinator commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2558941954)
I was pointing to this sentence in the PR description at the top, where it's still using "NO" rather than "OFF", as was changed in the code:
> By default the data at that location is embedded into the binary but there is also a build option to prevent this (`-DWITH_EMBEDDED_ASMAP=NO`).

(You can respond by jumping here: https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2514815293).

✅ laanwj closed an issue: "guix build fails on RISC-V due to python-py-cpuinfo test failure"
(https://github.com/bitcoin/bitcoin/issues/33873)

💬 laanwj commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3574292725)
Clearly a hardware issue, closing.

🤔 rkrux requested changes to a pull request: "wallet: Expand MuSig test coverage and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/33636#pullrequestreview-3503877587)
Thanks for accepting the refactoring suggestions!

💬 rkrux commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2559047569)
In b7215893e566dec8a085c08e722aa61ec5a2f5a0 "test: Add musig failure scenarios"

The `self.test_failure_case_3("mismatched key order", "tr(musig($0/<0;1>/*,$1/<1;2>/*))")` case seems incorrect to me because the participant pubkeys are sorted before generating the pubkey (and the address) that should result in both the wallets having same address.
https://github.com/bitcoin/bitcoin/blob/238c1c8933b1f7479a9bca2b7cb207d26151c39d/src/script/descriptor.cpp#L658

This test case is passing because
...

👍 TheCharlatan approved a pull request: "Add a "tx output spender" index"
(https://github.com/bitcoin/bitcoin/pull/24539#pullrequestreview-3503919394)
re-ACK 38f2f53963e833d25bc71df0713fb28cff17cc43

🤔 l0rinc reviewed a pull request: "contrib: turn off compression of macOS SDK to fix determinism (across distros)"
(https://github.com/bitcoin/bitcoin/pull/32009#pullrequestreview-3503804718)
I don't yet have experience with this part of the code, left a few nits.
I arrived here while investigating an upgrade to Xcode 16.3 in https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550717279 and
noticed a couple of potential issues around the `argparse` usage and the `-o`
handling.

I have tested the following changes locally:

```patch
diff --git a/contrib/macdeploy/gen-sdk.py b/contrib/macdeploy/gen-sdk.py
index 426d82e46c..3bf9154887 100755
--- a/contrib/macdeploy/gen-sdk.py
++
...

💬 l0rinc commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2559089068)
are these file extensions? I wasn't familiar with them, but e.g. https://medium.com/@mail2ashislaha/swift-objective-c-interoperability-static-libraries-modulemap-etc-39caa77ce1fc indicates they're extensions, in which case:

```suggestion
if tarinfo.name and tarinfo.name.endswith((".swiftmodule", ".modulemap")):
```

💬 l0rinc commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2559130969)
are we reassigning `out_sdkt_path` here? Based on the condition checking `args.out_sdkt`, shouldn't this rather be:
```suggestion
if args.out_sdkt:
out_sdkt_path = pathlib.Path(args.out_sdkt[0])
```

or maybe if we remove `nargs=1` from above we could do:
```suggestion
if args.out_sdkt:
out_sdkt_path = pathlib.Path(args.out_sdkt)
```

Was this working before? I don't really have experience with this, so I can't tell...

💬 l0rinc commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2558987253)
should we adjust the help description as well?
```suggestion
parser.add_argument("-o", metavar='OUTSDKTAR', nargs=1, dest='out_sdkt', required=False)
```

> usage: gen-sdk.py [-h] [-o OUTSDKTAR] XCODEAPP

👍 willcl-ark approved a pull request: "depends: sqlite 3.50.4; switch to autosetup"
(https://github.com/bitcoin/bitcoin/pull/32655#pullrequestreview-3504114386)
tACK 1db74914706fcfafb22646288458604a4a7b6282

This looks good to me. Tested a few random wallet operations in addition to the tests, which all seem ok.

Noting that whilst `--disable-dynamic-extensions` was dropped from configure options it's added as a preprocessor directive directly (although I think it could have been re-added to configure options as `--disable-load-extension`, but both are equivalent).

🤔 l0rinc reviewed a pull request: "RFC: bench: Add multi thread benchmarking"
(https://github.com/bitcoin/bitcoin/pull/33740#pullrequestreview-3504147480)
Not sure whether the concept of script validation has to creep into the benchmarking parameters, maybe we could generalize it to `-thread-count` or `-par` or something. Left a few nits.

💬 l0rinc commented on pull request "RFC: bench: Add multi thread benchmarking":
(https://github.com/bitcoin/bitcoin/pull/33740#discussion_r2559279775)
Script checks are currently the main source of threading, but not necessarily limited to this (e.g. https://github.com/bitcoin/bitcoin/pull/31132/commits or https://github.com/bitcoin/bitcoin/pull/26966 or https://github.com/bitcoin/bitcoin/pull/33689 or https://github.com/bitcoin/bitcoin/pull/32747), so I would suggest untangling multithreading from script validation here. If you insist this being about script validation (which sounds a bit too specific to include in a general benchmark), ignor
...

💬 l0rinc commented on pull request "RFC: bench: Add multi thread benchmarking":
(https://github.com/bitcoin/bitcoin/pull/33740#discussion_r2559261336)
hmmm, this looks like a refactor gone wrong, was this `arg.find("-worker-threads=") == 0` originally?

```suggestion
if (arg.starts_with("-worker-threads=")) {
```

so I guess `if (!found) {` should also be adjusted after this

---

Alternatively, instead of finding and overwriting, what if we unconditionally erased and pushed back the actual thread count, something like:
```C++
std::vector current_setup_args {args.setup_args};
if (threads > 0) {
std::erase_if(current_set
...

💬 l0rinc commented on pull request "RFC: bench: Add multi thread benchmarking":
(https://github.com/bitcoin/bitcoin/pull/33740#discussion_r2559321265)
What's the purpose of a vector over a range? Are we planning on testing threads `2,4,8,16` instead of `1,2,3,4,5,6...`?
If not, consider making it a range, something like:
```C++
constexpr int DEFAULT_MAX_BENCH_THREADS{16};
const auto [start_threads, end_threads]{is_thread_scaling ?
std::pair{1, DEFAULT_MAX_BENCH_THREADS} : std::pair{0, 0}};
```
and maybe iterate as
```C++
for (int threads{start_threads}; threads <= end_threads; ++threads) {
...
}
```

💬 l0rinc commented on pull request "RFC: bench: Add multi thread benchmarking":
(https://github.com/bitcoin/bitcoin/pull/33740#discussion_r2559283533)
What should happen when `worker_threads > MAX_SCRIPTCHECK_THREADS`?

💬 l0rinc commented on pull request "RFC: bench: Add multi thread benchmarking":
(https://github.com/bitcoin/bitcoin/pull/33740#discussion_r2559324072)
Might be out of scope for this PR, but instead of inlining `DEFAULT_BENCH_FILTER` here, can we move it over to `src/bench/bench.cpp`

```suggestion
bool is_thread_scaling = args.scale_threads && (args.regex_filter != DEFAULT_BENCH_FILTER);
```

🤔 rkrux reviewed a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3504404111)
ACK c34bc01b2ff2fc91ed4020288c5fa15f0c5b075e

⚠️ hulxv opened an issue: "RFC: Replacing `tinyformat` with `{fmt}`"
(https://github.com/bitcoin/bitcoin/issues/33942)
# Description

Bitcoin Core has used tinyformat (via its `strprintf` wrapper) for text formatting for many years, a choice that reflected the library’s simplicity and its ability to be dropped into a large C++ codebase with minimal friction. Over time, the C++ standard and the ecosystem around formatting have evolved: `{fmt}` has become the de facto modern formatting library and served as the basis for `std::format` in later C++ standards. At the same time, a security and maintenance review of T
...