💬 hodlinator commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#discussion_r2163882445)
Good find, but would rather leave that for another PR. This one has ballooned enough IMO just from wanting to remove `unique_ptr`.

💬 luke-jr commented on pull request "checkqueue: set MAX_SCRIPTCHECK_THREADS to nCores - 1":
(https://github.com/bitcoin/bitcoin/pull/32692#issuecomment-3001622066)
Maybe this is something best solved by runtime benchmarking? (Or perhaps just at startup, since early blocks may not give us the parallelization we need for it)

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3001627814)
Does it ever make sense to have multiple announcements for the same wtxid to be `m_reconsider`? I'm wondering if `AddChildrenToWorkSet` should skip wtxids for which a reconsider-announcement already exists.

💬 luke-jr commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2164763920)
```suggestion
std::string stable_version = strprintf("v%d.0", CLIENT_VERSION_MAJOR + 1);
```

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164783505)
Yes, that is a known and expected limitation. The purpose is to have cheap checks here as everything should already be checked by the parser. There is no way for these specific `PubkeyProvider`s to be created outside of this file anyways.

📝 hebasto opened a pull request: "cmake: Use `HINTS` instead of `PATHS` in `find_*` commands"
(https://github.com/bitcoin/bitcoin/pull/32805)
According to the CMake documentation, `HINTS` "should be paths computed by system introspection, such as a hint provided by the location of another item already found", which is precisely the case in the `FindQRencode` module.

Entries in `HINTS` are searched before those in `PATHS`. On macOS, Homebrew’s `libqrencode` will therefore be located at its real path rather than via the symlink in the default prefix.

A backport to 29.x is required for https://github.com/bitcoin/bitcoin/pull/32804,
...

💬 hebasto commented on pull request "Fix build on macOS when `qt@6` is installed":
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3001718898)
Please review https://github.com/bitcoin/bitcoin/pull/32805 first.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164821963)
Non-ranged participants generally won't be rederived every time because of the `read_cache`.

🤔 hodlinator reviewed a pull request: "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/31713#pullrequestreview-2955202681)
Pushed clang-tidy fix for <https://cirrus-ci.com/task/6094677570486272?logs=ci#L2068>:
```
[15:19:19.395] [ 29/669][11.0s] clang-tidy-20 -p=/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/src/test/fuzz/miniscript.cpp
[15:19:19.395] /ci_container_base/src/util/vector.h:23:49: error: function 'Vector<std::vector<miniscript::Node<(anonymous namespace)::ScriptParserContext::Key>>>' is within a recursive call chain [misc-no-rec
...

💬 real-or-random commented on pull request "test: disable secp256 tests by default":
(https://github.com/bitcoin/bitcoin/pull/32782#issuecomment-3001804058)
cc @sipa @jonasnick

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164866941)
Fixed

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164868678)
Since this `if` uses `ParsePubkey` recursively, I prefer to have it be in `ParsePubkey` than adding a circular dependency between `ParsePubkey` and a `ParseMusig`.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164868920)
Ended up dropping this funciton

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164869706)
Ended up extracting this entire derive type parsing into a separate function.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164870116)
Renamed

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164870481)
Added a `has_hardened` output parameter.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164871481)
I've changed it to an `Assume`. I don't really like having `assert`s in modules like descriptors.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164871650)
Done, mostly

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164871808)
Done, mostly

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164872003)
Done