💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684698968)
Did something like that.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1684699167)
I've added a comment to clarify.

💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1684699856)
> Note: This line isn't directly equivalent to previous code because it trims whitespace from the end of the string, not just the beginning, but the net effect is the same because any whitespace at the end of the string would be ignored anyway.

@ryanofsky yes, that's why my first pushed implementation added `LeftTrimStringView` in 8f4293d892358ce145eb9a099f2e5f256c4d9c73. But got pushback in https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675658574 and trimming the end of the strin
...

📝 theuni opened a pull request: "depends: Fix CMake-generated `libevent*.pc` files"
(https://github.com/bitcoin/bitcoin/pull/30488)
Broken out of #30454. This is a backport of the merged upstream PR: https://github.com/libevent/libevent/pull/1622.

Note that after #29835 we might end up dropping pkg-config and using the installed CMake files directly, but that depends on whether or not enough distros actually ship those files.

Either way, having fixed up .pc files won't hurt.

👍 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