💬 willcl-ark commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2904253640)
Pushed an update which now only activates the dependency provider when using the depends toolchain.

This will hopefully appease the windows CI job, as well as making more sense in general and being more expected behaviour.

🤔 janb84 reviewed a pull request: "wallet, rpc: Return normalized descriptor in parent_descs"
(https://github.com/bitcoin/bitcoin/pull/32594#pullrequestreview-2864259747)
ACK 107517ea7d29d959e7a0b94d8217dcb894680547

Before change listtransactions (and others) shows un-normalized descriptors, after the descriptors are normalized


<details>
master:

```
{
"address": "bcrt1qgymj02gx6dzrkh49d9sj0cj8rxfcrut7dyxz7r",
"parent_descs": [
"wpkh(tpubD6NzVbkrYhZ4YdzwuYwQLEypddJ64KVEG9tyZibLxLKMxyMCaru1c7RFX2S8NEQ7dZuNccdzk5qikw51rxzXYWdkeuREgxgdqbJy3ty68qy/84h/1h/0h/0/*)#tz7s9af2"
],
"category": "immature",
"amount": 50.00000000,
...

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2104498772)
Thanks! Fixed in https://github.com/bitcoin/bitcoin/pull/32540/commits/9d7e23e2f505ce6cbe830fc607cf203b1a48ba0d.

📝 hebasto opened a pull request: "test: Fix `system_tests/run_command` test"
(https://github.com/bitcoin/bitcoin/pull/32601)
This PR addresses two issues related to the "Return non-zero exit code, with error message for stderr" check in the `system_tests/run_command` test:
1. The test now checks the exact exit code, which must be 1.
2. The process invocation string is removed from the exception message before checking the stderr output, as the latter is a substring of the former.

This is an alternative to https://github.com/bitcoin/bitcoin/pull/32577 without introducing a shell wrapper and altering quotation in `
...

✅ hebasto closed a pull request: "subprocess: Let shell parse command on non-Windows systems"
(https://github.com/bitcoin/bitcoin/pull/32577)

💬 hebasto commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2904327201)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/32601.

💬 hebasto commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#discussion_r2104533575)
https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2902382891:

> - I don't think the added quotes in external_signer.cpp are doing anything, or it isn't clear what they are trying to do.

Descriptors contain the `(` and `)` characters, which may cause issue with shell:
```
$ echo pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)
bash: syntax error near unexpected token `('
```

💬 maflcko commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104545752)
This will just re-introduce the bugs fixed in https://github.com/bitcoin/bitcoin/pull/31542, no?

💬 maflcko commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104546796)
A better option would be to use the test's temp dir as scratch space

💬 hebasto commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104550766)
> This will just re-introduce the bugs fixed in #31542, no?

Oops, forgot about it :facepalm:

📝 hebasto converted_to_draft a pull request: "test: Fix `system_tests/run_command` test"
(https://github.com/bitcoin/bitcoin/pull/32601)
This PR addresses two issues related to the "Return non-zero exit code, with error message for stderr" check in the `system_tests/run_command` test:
1. The test now checks the exact exit code, which must be 1.
2. The process invocation string is removed from the exception message before checking the stderr output, as the latter is a substring of the former.

This is an alternative to https://github.com/bitcoin/bitcoin/pull/32577 without introducing a shell wrapper and altering quotation in `
...

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2104493385)
in commit 35db4f2dcfc3435e10935581ffa447ffe219cc1e: nit, could use the helper here:
```suggestion
if (IsRangedDerivation() || !m_path.empty()) {
```

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2104489088)
in commit 35db4f2dcfc3435e10935581ffa447ffe219cc1e: the byteswap comment is obsolete now
```suggestion
//! MuSig2 chaincode as defined by BIP 328
using namespace util::hex_literals;
constexpr uint256 MUSIG_CHAINCODE{"868087ca02a6f974c4598924c36b57762d32cb45717167e300622c7167e38965"_hex_u8};
```

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2104518220)
in commit 739f13986a49d8d7d5568ebcdd88665ea8423d42: could add a doxygen comment for the newly introduced parameter

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2104576393)
Why is the `key_exp_index` parameter changed to pass by reference? Without that the unit tests fail, so it seems necessary, but I'm confused why this was not needed earlier already for similar constructs (e.g. for `multi()` expressions where the value is also incremented in the key expression parsing loop). Probably I'm missing some basic descriptors parsing knowledge.

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2104615891)
@hebasto compelling enough to ACK as is?

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2104621096)
Took me longer than anticipated, but I have measured different scenarios here to understand where alignment matters and where it doesn't, with gcc, clang and apple clang

GCC 14 is 38.5% faster with alignment & 64 byte unroll compared to just a 64 bit xor

<details>
<summary>------ C++ compiler .......................... GNU 14.2.0 ------</summary>

```bash
rm -rf build && cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ >/dev/nul
...

👍 theStack approved a pull request: "Musig2 fields followups"
(https://github.com/bitcoin/bitcoin/pull/32412#pullrequestreview-2864492651)
ACK 10fd2aeb62469510c115362f801c08563421c304

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2104639128)
Done

💬 pinheadmz commented on issue "getchaintips doesn't mark headers-only chain as invalid":
(https://github.com/bitcoin/bitcoin/issues/8050#issuecomment-2904516982)
I believe this issue was closed by https://github.com/bitcoin/bitcoin/pull/30666

@mzumsande