👍 tdb3 approved a pull request: "test: add creating/spending validity checks for rare output scripts"
(https://github.com/bitcoin/bitcoin/pull/30481#pullrequestreview-2188793231)
ACK 006d835acf98ebceaaaead7aea41f512ed5023ad
Seems like a good addition. Observed a runtime of around 1s, so minimal.
Left some thoughts. Will also think about other potential candidate rare output scripts and comment if any come to mind.

💬 tdb3 commented on pull request "test: add creating/spending validity checks for rare output scripts":
(https://github.com/bitcoin/bitcoin/pull/30481#discussion_r1684707058)
Thinking out loud:
Initially thought of ensuring the edge cases (2 and 40) are explicitly added in addition to random program size selection, but this would probably be overkill.

💬 tdb3 commented on pull request "test: add creating/spending validity checks for rare output scripts":
(https://github.com/bitcoin/bitcoin/pull/30481#discussion_r1684710348)
Thinking out loud:
Might be nice to enhance ECPubKey to handle hybrid keys (prevents having to do byte manipulation here). Seems outside the scope of this PR though.

📝 theuni opened a pull request: "depends: zmq: Fix Autotools-generated `libzmq.pc` file"
(https://github.com/bitcoin/bitcoin/pull/30489)
Similar to #30488. Broken out of #30454. This is just taking the (merged) upstreamed PR: https://github.com/zeromq/libzmq/pull/4667

On top of #29723 as that would otherwise conflict and is likely to be merged soon. I'll rebase here and undraft after that's merged.

Like #30488, this may be obsoleted soon after switching to building with CMake because we'll be able to use those files rather than pkg-config, but again it doesn't hurt to have the fixed files.

✅ theuni closed a pull request: "depends: zmq: Fix Autotools-generated `libzmq.pc` file"
(https://github.com/bitcoin/bitcoin/pull/30489)

💬 theuni commented on pull request "depends: zmq: Fix Autotools-generated `libzmq.pc` file":
(https://github.com/bitcoin/bitcoin/pull/30489#issuecomment-2239844520)
Hah, I realized as soon as I hit the button that #29723 obsoletes this. Either the .pc is fine there, or it needs to be patched in a different place. Closing.

💬 theuni commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2239864170)
Looking at the mingw .pc generated by this PR:
```
Libs: -L${libdir} -lzmq
Libs.private:
Requires.private:
```

It looks like we'll need to take https://github.com/zeromq/libzmq/pull/4706 as well for CMake. That can be done as a follow-up though, as it's not yet merged upstream.

💬 theuni commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1684802528)
Looks like this can be bumped further now that https://github.com/chaincodelabs/libmultiprocess/pull/98 has been merged?

📝 theuni opened a pull request: "depends: bump libmultiprocess for CMake fixes"
(https://github.com/bitcoin/bitcoin/pull/30490)
Broken out of #30454 . Bumped [even further](https://github.com/bitcoin/bitcoin/pull/30454/commits/4883197abc63aedbc395f37f6d2aded5db5270aa#r1684802528) after https://github.com/chaincodelabs/libmultiprocess/pull/98 was merged upstream.

@hebasto Presumably this approach works now with the CMake branch?

💬 theuni commented on pull request "depends: bump libmultiprocess for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30490#issuecomment-2239894232)
Ping @ryanofsky for a quick concept ACK for bumping to this particular commit.

✅ Sjors closed a pull request: "Stratum v2 connman"
(https://github.com/bitcoin/bitcoin/pull/30332)

💬 Sjors commented on pull request "Stratum v2 connman":
(https://github.com/bitcoin/bitcoin/pull/30332#issuecomment-2239955808)
I moved this PR to my own fork at https://github.com/Sjors/bitcoin/pull/50 to reduce CI load, which apparently was becoming too much: https://github.com/bitcoin/bitcoin/pull/30475#issuecomment-2238759084

✅ Sjors closed a pull request: "Stratum v2 Template Provider common functionality"
(https://github.com/bitcoin/bitcoin/pull/30475)

💬 Sjors commented on pull request "Stratum v2 Template Provider common functionality":
(https://github.com/bitcoin/bitcoin/pull/30475#issuecomment-2239959662)
Managed to fix the CI issue.

Moving this PR to https://github.com/Sjors/bitcoin/pull/49 to reduce the PR stack here a bit and not overwhelm CI.

👍 hebasto approved a pull request: "depends: Fix CMake-generated `libevent*.pc` files"
(https://github.com/bitcoin/bitcoin/pull/30488#pullrequestreview-2189047606)
ACK 8c935e625ea75d180144f0526d6a0d5fd58c1f29.

👍 ryanofsky approved a pull request: "depends: bump libmultiprocess for CMake fixes"
(https://github.com/bitcoin/bitcoin/pull/30490#pullrequestreview-2189098956)
Code review ACK d318c4ef56465ccad1a1d4d27c52216e0b69ad4e.

I haven't tested the cmake support, but I made the same version bump in 3e6c61fdc2839bdb74563538aaf0a5e7d0e07ea3, which is part of #10102 and 30437

💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#issuecomment-2240082764)
Thanks for the feedback! Helped me realize I was being too terse. (I think it was a mistake alone just to not have `string_view` in the title for the sake of keeping under 51 chars).

Only modified commit message:
```diff
--- c3a9c71c7077324bf0aa40f834f7537a14619340 2024-07-19 22:42:56.019823786 +0200
+++ 649c88d4df0c97cd0109c0cd0e54bc76f2287ef2 2024-07-19 22:42:57.539661157 +0200
@@ -1,3 +1,5 @@
-fix: Make TxidFromString() respect string length
+fix: Make TxidFromString() respect string
...

💬 hodlinator commented on pull request "refactor: Add consteval uint256("str") and ParseHex("str")":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1684964695)
I think it's more acceptable to diverge on behavior if we call the `consteval` function something different, see https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1682782077

📝 hebasto opened a pull request: "Fix MSVC warning C4273 "inconsistent dll linkage""
(https://github.com/bitcoin/bitcoin/pull/30491)
Broken out of https://github.com/bitcoin/bitcoin/pull/30454.

When using CMake, the user can select the MSVC runtime library to be:
1) Statically-linked (with the corresponding `x64-windows-static` vcpkg triplet) or
2) Dynamically-linked (with the corresponding `x64-windows` vcpkg triplet)

In the latter case, the compiler emits the [C4273](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4273) warning.

As the "Necessary on some platforms"
...

💬 hebasto commented on pull request "Fix MSVC warning C4273 "inconsistent dll linkage"":
(https://github.com/bitcoin/bitcoin/pull/30491#issuecomment-2240116123)
Friendly ping:
- @theuni per offline request for this PR
- @sipsorcery as a Windows connoisseur
- @sipa as an author of the [original code](https://github.com/bitcoin/bitcoin/commit/2554c1b81bb8c40e1989025c6f18e7935720b156)