💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681211287)
The missed EOL causes "Hunk #4 succeeded at 1540 with fuzz 1."

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681214480)
```suggestion
@@ -599,7 +599,7 @@ if(NOT MSVC)
```

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681211106)
nit: missed EOL.

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681210901)
The missed EOL causes "Hunk #1 succeeded at 7 with fuzz 1."

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681238832)
Unless this patch is submitted upstream, why not use the same version as our CMake-based build system?

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681232558)
Cannot find b163fc36a48ecdc87a3ecb4c6bba5f6b8965acaf in https://github.com/zeromq/libzmq. Did you mean https://github.com/zeromq/libzmq/commit/aa885c5a154256612108636b0fb22f44ae0e247a?

Perhaps we could refer to a PR number, as is done in other patches?

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681251382)
```suggestion
@@ -564,13 +564,6 @@ else()
```

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681252005)
```suggestion
@@ -588,9 +581,7 @@ if(WIN32 AND NOT CYGWIN)
```

💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2233601603)
> Hmm, what specifically would you like me to change here?

Just to have the changes in `src/ipc/capnp` be their own commit. I need those in https://github.com/Sjors/bitcoin/pull/48, while at the same I need to avoid including the rest of 4e1a4342f3b2a857e3211587cd14e36704197483.

> The purpose of createNewBlock2 is just to make sure serialization works when createNewBlock is changed to return BlockTemplate.

In that case I'll just add a commit to https://github.com/Sjors/bitcoin/pull/48
...

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681266250)
The current condition (for using DFS) is:

```c++
while (queue.size() - 1 + queue.front().und.Count() > queue.capacity()) {
```

In the original version of this PR it was:

```c++
const auto queuesize_for_front = queue.capacity() - queue.front().und.Count();
...
while (queue.size() > queuesize_for_front) {
```

There is an offset 1 difference between the two; it didn't take into account that choosing BFS would first delete an entry from the queue before adding new ones.

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1681267687)
yes this was very insightful, thank you

💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r1681269853)
Responded in https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2233601603

💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2233625152)
Updated 4e1a4342f3b2a857e3211587cd14e36704197483 -> e1ae38f4bbcdb1b3fa5e1eab1abfa9cf2232798c ([`pr/mine.3`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.3) -> [`pr/mine.4`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.3..pr/mine.4)) adding several unit tests, and also starting to split up the "Add bitcoin-mine test program" commit by moving CTransaction serialization support to a new commit.

👍 theuni approved a pull request: "Fix SSE4.1-related issues"
(https://github.com/bitcoin/bitcoin/pull/28893#pullrequestreview-2183328324)
utACK d440f13db02c82c842000abe4fe4d0c721a4ad3b

💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2233652640)
Updated e1ae38f4bbcdb1b3fa5e1eab1abfa9cf2232798c -> e1fa475a5becca0083f59d7cafac9bc67c2f4f1c ([`pr/mine.4`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.4) -> [`pr/mine.5`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.5), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.4..pr/mine.5)) adding support for -loglevel and other logging parameters as suggested https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2233469862

💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1681303764)
I doubt upstream is going to take any patches. We can just set it to 3.22.

🤔 BrandonOdiwuor reviewed a pull request: "rpc, rest: Improve getblock error when only header is available"
(https://github.com/bitcoin/bitcoin/pull/30410#pullrequestreview-2183351347)
Concept ACK

💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2233661982)
Rebased. Added all the missing newline/diff adjustments. Swapped CMake minimum patch to 3.22. Updated the PR reference in the Windows patch.

✅ fanquake closed an issue: "build: Configuring with `-mno-sse4.1` does not fail the sse4.1 instrinsics check"
(https://github.com/bitcoin/bitcoin/issues/28864)

🚀 fanquake merged a pull request: "Fix SSE4.1-related issues"
(https://github.com/bitcoin/bitcoin/pull/28893)