💬 rkrux commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2123692569)
```diff
- aspostrophe
+ apostrophe
```

💬 rkrux commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2123691895)
I checked that this Regex is fine because the origin fingerprint can only be a hex string `(\da-f)` and the `origin_part` is found even if the `desc_verify` doesn't contain hardened marker such as in `wpkh([58d258be/84/1/0]/0/*`. That's why I believe the subsequent `"h" in origin_part` check is required.

💬 rkrux commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2123713913)
`r"tpub\w+?(?=/)"`

The `\w` will accept an xpub with underscore as well that is not allowed, but ok.

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

Tested that this works. I additionally tested rebasing #32595 on it, where it also works well.

💬 willcl-ark commented on pull request "depends: Bump boost to 1.88.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/32665#issuecomment-2935137626)
guix hashes:

```
src/core/bitcoin on  pr-32665 [$] via △ v3.31.6 via 🐍 v3.12.10 via ❄️ impure (nix-shell-env) took 50m49s
❯ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
32f567efe7c9d428083d9eab15e9fb215e2860132b3a31a60f288d21d3ed3644 guix-build-3a350c8a1b51/output/aarch64-linux-gnu/SHA256SUMS.part
13bfeec126cef80d97a7b90f9cec32962cce8b3bbf445006f1233e1b45a74d43 guix-build-3a350c8a1b51/output/aarch64-linux-gnu/bi
...

📝 maflcko opened a pull request: " clang-tidy: Apply modernize-deprecated-headers "
(https://github.com/bitcoin/bitcoin/pull/32673)
Bitcoin Core is written in C++, so it is confusing to sometimes use the deprecated C headers (with the `.h` extension). For example, it is confusing, when it is less clear whether `string.h` refers to the file in this repo or the cstring stdlib header (https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121492797).

The check is currently disabled for headers, to exclude subtree headers.

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2123760984)
in commit 9e3a1d05bf066df31a6e124638046e70669eb470: nit: the `Expr` call seems to be a no-op here, considering that we already extracted the function expression with the parens-splitting above
```suggestion
std::span<const char> expr(split.at(0).begin(), split.at(0).end());
if (!Func("musig", expr)) {
```

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2123695013)
in commit 16a88829e92c2676cae6b6e0acb75d2d26795a6d:
```suggestion
//! Derivation path
const KeyPath m_path;
```
as using non-ranged derivation is also possible (e.g. `musig(...)/1`, we also have a [test for that](https://github.com/achow101/bitcoin/blob/afcd397bc791d047508adb9d151d21384d7b49e3/src/test/descriptor_tests.cpp#L1118)); the BIP 390 specification can be interpreted in a way that only ranged derivation is possible though (https://github.com/bitcoin/bips/blob/master/bip-039
...

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2123808039)
in commit 9e3a1d05bf066df31a6e124638046e70669eb470: nit: A more thorough check would be to detect duplicates of actual resulting pubkeys, rather than only the expression strings (i.e. for a keypair `(priv,pub)`, the expression `musig(priv,pub)` would now still be valid).

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2123779414)
in commit 9e3a1d05bf066df31a6e124638046e70669eb470: should this be
```suggestion
if (!all_bip32) {
error = "musig(): musig() derivation requires all participants to be xpubs";
```
instead? Ranged would mean ending with `/*` IIUC, which is not necessarily the case.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2123834199)
I believe that if somebody is using `-debug=bench` or `-debug=leveldb`, the fact that they are using a category should bypass the rate limit if `LogDebug` is used. I think the mitigation should be in place whether or not we're in IBD -- if we have any inbound peers, we need to be cautious.

💬 maflcko commented on pull request "doc: Remove build instruction for running `clang-tidy`":
(https://github.com/bitcoin/bitcoin/pull/32662#issuecomment-2935293250)
Shouldn't this be excluded in the tidy CI task as well?

Also, would be good to mention that this will now print errors about the missing auto-generated files.

⚠️ fanquake opened an issue: "guix: nsis (Windows installer creator) is broken upstream"
(https://github.com/bitcoin/bitcoin/issues/32674)
This will become a issue when we want to bump the time-machine next, and it'd be nicer if this is fixed and shipped upstream, rather than us having to work around the issue:

```bash
# guix --version
guix (GNU Guix) 4b9d14378fcc3d8dd4eea36b541fe87e198fd7b8

# guix build --no-substitutes nsis-x86_64
Source/Plugins.cpp:278:18: required from ‘static void PrintPluginDirsHelper::print(const C&, const char*) [with C = std::map<std::__cxx11::basic_string<wchar_t>, std::__cxx11::basic_string<wchar_t>,
...

💬 pinheadmz commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2935375333)
@achow101 re: slow preferred peer "causing congestion" -- wouldn't this only affect the single user who explicitly set the preferred peer?

💬 pinheadmz commented on issue "Potential data race on fHavePruned flag":
(https://github.com/bitcoin/bitcoin/issues/21627#issuecomment-2935386395)
I haven't been focusing on this, wanna take a stab at it?

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2123924524)
> Is there an issue open for this upstream?

Please see https://github.com/include-what-you-use/include-what-you-use/issues/1763.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2123928871)
The issue could be worked around in different ways. For example, by using an OS image with the default GCC >= 14.

💬 hebasto commented on issue "guix: nsis (Windows installer creator) is broken upstream":
(https://github.com/bitcoin/bitcoin/issues/32674#issuecomment-2935438865)
Does it depend on the build machine architecture?

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2123954582)
See https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115570067, will change. Imo, having to determine whether we are in IBD or have zero peer connections is heading into too much complexity. This PR shouldn't cause IBD debug logs to be rate-limited, but if it does we should fix that.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#issuecomment-2935446085)
Friendly ping @fjahr and @polespinasa for reviewing some HTTP tests ? <3

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2123959106)
This never gets cleared. I haven't calculated the overhead of the map, but it could make sense to clear it periodically.