👍 theStack approved a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2843083473)
re-ACK ee045b61efc1479c1866b786661ef39a863677d0
(as per `$ git range-diff f09e1f94...ee045b61`)

⚠️ fanquake opened an issue: "test: feature_framework_startup_failures.py fails with `--timeout-factor=0`"
(https://github.com/bitcoin/bitcoin/issues/32506)
master @ c779ee3a4044df3a263394bbb8b104aeeaa7c727
```bash
./build/test/functional/test_runner.py feature_framework_startup_failures.py --timeout-factor=0
Temporary test directory at /tmp/test_runner_₿_🏃_20250515_101739
Remaining jobs: [feature_framework_startup_failures.py]
1/1 - feature_framework_startup_failures.py failed, Duration: 1 s

stdout:
2025-05-15T10:17:39.659000Z TestFramework (INFO): PRNG seed is: 8331063254222734517
2025-05-15T10:17:39.660000Z TestFramework (INFO): Initializing te
...

💬 fanquake commented on issue "test: feature_framework_startup_failures.py fails with `--timeout-factor=0`":
(https://github.com/bitcoin/bitcoin/issues/32506#issuecomment-2883310880)
cc @hodlinator

💬 laanwj commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883318635)
> @laanwj you might remember / know of such systems?

Yes. At the time there was cloudabi, which was a sandboxed version of POSIX that removed all contextual access and ambient authority, so processes only get capability handles passed in to what they need. So there was no shell and hence no `std::system`. But that project was abandoned years ago.

i'm not sure any other such platforms exist right now, i'm fine with removing the check. But adding a comment makes sense nevertheless.

Aside:
...

💬 l0rinc commented on pull request "refactor: Remove UB in prevector reverse iterators":
(https://github.com/bitcoin/bitcoin/pull/32490#issuecomment-2883326795)
This PR is an extension of https://github.com/bitcoin/bitcoin/pull/32296 (which originally just removed the unnecessary `prevector` suppression, but @maflcko bisected the cause and removed the dead code as well).
Your reviews would be appreciated there as well.

💬 hebasto commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883331380)
> Aside: i would like to eventually convert usage of `std::system` to `subprocess`, especially now (after #32343) that it doesn't leak file descriptors anymore.

:+1:

💬 hebasto commented on pull request "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2883341081)
This issue seems related: https://developercommunity.visualstudio.com/t/Debug-CRT-shows-abort-message-box-instea/10904782.

👍 rkrux approved a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2843186024)
re-ACK ee045b61efc1479c1866b786661ef39a863677d0

Only rebase changes from #32305
```
git range-diff f09e1f94...ee045b61
```

💬 hodlinator commented on issue "test: feature_framework_startup_failures.py fails with `--timeout-factor=0`":
(https://github.com/bitcoin/bitcoin/issues/32506#issuecomment-2883371822)
Thanks for finding & reporting. I've been preparing a follow-up PR for the one that introduced the test (#30660) last week, will add a commit to resolve this issue there.

💬 maflcko commented on issue "test: wallet_reorgsrestore.py failure under valgrind":
(https://github.com/bitcoin/bitcoin/issues/32493#issuecomment-2883411301)
I guess it is the same reason that feature_init.py was disabled in the CI only

📝 maflcko opened a pull request: "ci: Exclude failing wallet_reorgsrestore.py from valgrind task for now"
(https://github.com/bitcoin/bitcoin/pull/32507)
Fixes https://github.com/bitcoin/bitcoin/issues/32493

For some reason terminate or kill do not work inside the CI system under valgrind.

So disable the test for now, until a solution is found.

💬 fanquake commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883423011)
Guix Build:
```bash
bcf993e048118c80beae601700c50f8d707474b0e5f89359dfc8781a12efee81 guix-build-c0f41523973e/output/aarch64-linux-gnu/SHA256SUMS.part
a6cf6122393f86b8a34e036b8a237daa3153613d154eae2f8c66720c8bad0b52 guix-build-c0f41523973e/output/aarch64-linux-gnu/bitcoin-c0f41523973e-aarch64-linux-gnu-debug.tar.gz
3330a1a5848fa80f24b181f8fe22341dca8abe9a1934a8c75834f51b311ad780 guix-build-c0f41523973e/output/aarch64-linux-gnu/bitcoin-c0f41523973e-aarch64-linux-gnu.tar.gz
eb8ba4643c8b2774
...

⚠️ fanquake opened an issue: "ci: remove third-party javascript usage from Windows CI"
(https://github.com/bitcoin/bitcoin/issues/32508)
See:

https://github.com/bitcoin/bitcoin/blob/c779ee3a4044df3a263394bbb8b104aeeaa7c727/.github/workflows/ci.yml#L189-L190

We shouldn't need to use a third-party repo, that runs some Javascript, to configure a command prompt. The comment in our code also doesn't explain why this is necessary. We should be able to drop this dependency.

Also discussed in #32396.

💬 hebasto commented on pull request "doc: Add hint about avoiding spaces in paths when building on Windows":
(https://github.com/bitcoin/bitcoin/pull/32397#issuecomment-2883438040)
> What is the status of this? Do we need to add something to the docs?

I believe it would be helpful to mention a workaround in the build documentation. [Here](https://github.com/hebasto/bitcoin/commit/6a2a5428346634c80e6856e17b2c737bb523fd14)'s a suggestion you could consider including.

Feel free to grab it.

cc @davidgumberg

💬 fanquake commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2090923730)
Opened #32508 to follow up with the CI.

💬 brunoerg commented on pull request "test: descriptor: cover invalid multi/multi_a cases":
(https://github.com/bitcoin/bitcoin/pull/32504#issuecomment-2883461802)
> Nice, but two commits for adding three lines of related tests seems like it could be squashed?

Done.

👍 fanquake approved a pull request: "ci: Exclude failing wallet_reorgsrestore.py from valgrind task for now"
(https://github.com/bitcoin/bitcoin/pull/32507#pullrequestreview-2843310006)
ACK fa981b90f53101bff2eda606d9479233e71736b5

💬 hodlinator commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2883487720)
nit: Should PR description include "ref #31303"?

Tried removing the `compiler_has_bug`-logic completely and building with the version I had installed: [17.13.4](https://learn.microsoft.com/en-us/visualstudio/releases/2022/release-notes-v17.13#17.13.4)...

```
cmake -B build --preset vs2022 -DVCPKG_INSTALL_OPTIONS="--x-buildtrees-root=C:\vcpkg" -DBUILD_FUZZ_BINARY=ON -DBUILD_GUI=OFF
cmake --build build --config Release -j16
```
...did not encounter the internal compiler error?

Couldn'
...

💬 fanquake commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883491267)
This seems to be broken for mac cross builds:
```bash
make -C depends HOST=x86_64-w64-mingw32
cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
<snip>
CMake Error at cmake/module/AddWindowsResources.cmake:9 (target_sources):
Cannot find source file:

/Users/xxx/bitcoin/src/bitcoin-util-manifest.rc
Call Stack (most recent call first):
cmake/module/AddWindowsResources.cmake:25 (add_windows_resources)
src/CMakeLists.txt:427 (add_windows_application_manifest)
...

💬 maflcko commented on pull request "test: descriptor: cover invalid multi/multi_a cases":
(https://github.com/bitcoin/bitcoin/pull/32504#issuecomment-2883537760)
lgtm ACK 58e55b17e632dbd4425dd64825b087f242ac4b7b