💬 Sjors commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1746681102)
I ended up making separate functions in 6f5419e3953781ade75d7986f293ce0fc13e90ce.

💬 Sjors commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2333484176)
Thanks for the review, I took the suggestions.

It could be useful to add version grinding support to OP_TRUE signets.

💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746694411)
it's not the counting, it's to document that this version doesn't have negatives (like the excess version does)

💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746696954)
What's the advantage of the `string_view` version?

💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2333515870)
> Maybe split [187ba68](https://github.com/bitcoin/bitcoin/commit/187ba684a9d5e63d09b43511b7128cce9edbedf3) into one commit that moves the implementation and another that splits the class.

`e59097a0a5...e995ffa5c3`: do that ^

The cumulative diff before and after this push is identical:

```sh
$ diff -u <(git diff e59097a0a5~2..e59097a0a5) <(git diff e995ffa5c~3..e995ffa5c)
$
```

💬 maflcko commented on pull request "interpreter: use int32_t instead of int type for risczero compile":
(https://github.com/bitcoin/bitcoin/pull/30794#issuecomment-2333518330)
review-only ACK bc52cda1f3c007bdf1ed00aa3011e207c7531017

Changing `int` to `int32_t` in Bitcoin Core should always be fine, because they are assumed to be of the same bit-width and bit-representation. Generally, it is even preferred to use `int32_t`, especially in serialization code, because it documents the width explicitly.

However, I haven't tested this or reviewed that the patch is complete. It would be nice if there was an easy way to highlight all places in the codebase where distinc
...

💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1746705958)
Just stylistic - I though it would be better to have it before the classes declarations. Should I move it at the bottom, where it was?

💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746708893)
> important thing is the error, not the exact wording of the error message

You have separate error for trailing `%`, why not have dedicated errors for when the format specifiers are fewer/more than the args?

💬 mzumsande commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2333526650)
> Let’s say you have Peer1 and Peer2 announcing the Transaction ABC, if ABC is concurrently fetch from both Peer1 and Peer2, the `GETDATA` (32-bytes) for ABC would have been wasted as outgoing bandwidth by Local Node. This 32 byte leak can be amplified if batch of transactions are concurrently fetched (bounded by `_INVENTORY_BROADCAST_INTERVAL`).

That has nothing to do with free relay, because it doesn't propagate to the rest of the network (hence the "relay" in the name). It's also not a int
...

👍 l0rinc approved a pull request: "util: Use consteval checked format string in FatalErrorf"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2285402162)
While I would still prefer some of our comments to be considered, I'm also fine with merging the change as is - thanks for making the developer experience more streamlined via these small-but-useful checks!

ACK fa092749094aa483e3ce0243885ce2eb8ed22cbb

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746712867)
Just being a bit more explicit. A single `s` character may otherwise be missed or removed in the future, leading to a raw pointer compare, which would be wrong.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746714182)
> why not

The reason is in my previous reply.

✅ fanquake closed an issue: "cmake: Step "-- Looking for C++ include boost/test/included/unit_test.hpp" takes a long time"
(https://github.com/bitcoin/bitcoin/issues/30787)

🚀 fanquake merged a pull request: "cmake: scope Boost Test check to `vcpkg`"
(https://github.com/bitcoin/bitcoin/pull/30822)

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746719104)
Ok, in that case I'd prefer `unsigned count{0};`. I may switch to that, if I have to re-touch.

💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746720865)
ok, that's fair, thanks.

💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746725437)
I saw your previous response, I was quoting from it in mine. Anyway, you can resolve this comment.

🚀 fanquake merged a pull request: "test: Pin and document TEST_DIR_PATH_ELEMENT, SeedRand::FIXED_SEED"
(https://github.com/bitcoin/bitcoin/pull/30748)

💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2333551090)
Can you rebase this for CMake?

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2333554993)
re-utACK e225f7cbc3e6842f8e7f1c482c2aacd810e99c1b

I plan to test this with https://github.com/Sjors/bitcoin/pull/48 but atm the rebases involved are a bit too much headache.