💬 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.

💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#issuecomment-2333558016)
> In tinyformat there is no such thing, so I'll leave this as-is for now.

My motivation is not to reach parity with *tinyformat*, it is to at least cover some of what the prior linter did:

https://github.com/bitcoin/bitcoin/blob/c3af4b1ec3fdb308404199ddd0df5170793a2c39/test/lint/run-lint-format-strings.py#L230-L253

An alternative to making it an error is to simply make invalid chars not count, which is closer to the old behavior. Would that be okay?

If you don't want to bring over at
...

💬 Sjors 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_r1746733558)
Not a big deal, but diff was already quite large.

💬 maflcko commented on pull request "test: Pin and document TEST_DIR_PATH_ELEMENT, SeedRand::FIXED_SEED":
(https://github.com/bitcoin/bitcoin/pull/30748#issuecomment-2333560129)
> IMO, `TEST_DIR_PATH_ELEMENT` would be clearer as `TESTS_DIR_NAME` and `"test_common bitcoin"` would be clearer as `"bitcoin tests"` but current names seem fine too.

Looks like https://github.com/bitcoin/bitcoin/pull/30737 may or may not change the path layout, so any clarifications could in theory be done there.

💬 fanquake commented on issue "ci: failure in win64 unit tests":
(https://github.com/bitcoin/bitcoin/issues/30792#issuecomment-2333563239)
I wonder if it was an intermittent `wine` related issue, but the output was missing. Similar to something like: https://cirrus-ci.com/task/6309656165875712.

🚀 fanquake merged a pull request: " bench: Remove redundant logging benchmarks "
(https://github.com/bitcoin/bitcoin/pull/30790)

💬 Sjors 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-2333566726)
@vasild it's probably easier to reverse those two commits: first move the implementation out, and then split the class.

🤔 TheCharlatan reviewed a pull request: "interfaces: #30697 follow ups"
(https://github.com/bitcoin/bitcoin/pull/30828#pullrequestreview-2285497604)
Concept ACK

💬 TheCharlatan commented on pull request "interfaces: #30697 follow ups":
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1746748313)
How about making the parameter `&&` to avoid accidental copies?