🤔 ryanofsky reviewed a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2402142576)
Updated 1d16d6e6bac994fed7c695f530b9984edcd290bd -> e6086e00e32e486aaeeeb346ccca1377bbf647b2 ([`pr/tcheck.1`](https://github.com/ryanofsky/bitcoin/commits/pr/tcheck.1) -> [`pr/tcheck.2`](https://github.com/ryanofsky/bitcoin/commits/pr/tcheck.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/tcheck.1..pr/tcheck.2)) addressing comments and making `ConstEvalFormatString` parsing stricter to reject incomplete specifiers.

💬 theuni commented on pull request "consensus: fix `OP_1NEGATE` handling in `CScriptOp`":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2444682990)
I'm ~0 on this leaning towards NACK. As far as I can tell, all code paths are currently gated by `if (>= OP_1 && <= OP_16)` when calling `DecodeOP_N`/`DecodeOP_N`.

So this is really only changing the intention of those functions. And since it would be unsafe (because of `OP_RESERVED`) to change those gates in the callers to `>= OP_1NEGATE && <= OP_16`, it seems like `OP_1NEGATE` is going to have to remain a special case for callers anyway.

💬 fanquake commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#issuecomment-2444700876)
We should also follow up with refactoring the libevent module, to more generically use CMake/pkg-config, rather than restricting the CMake usage to `vcpkg`. At that point, we'd likely be able to dump pkg-config for the depends path entirely.

👍 fanquake approved a pull request: "cmake: Add `FindZeroMQ` module"
(https://github.com/bitcoin/bitcoin/pull/30903#pullrequestreview-2402413240)
ACK 915640e191b6a17a245f0502bc399d82a6502ccf

💬 hebasto commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2444707227)
re: https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2437357680

> It would be good if ctest/Windows actually printed the exception and error message instead of silently hiding it.

I'm not aware of the specific case discussed above, but exceptions in tests are generally printed as follows (with a patched code):
```
> ctest --test-dir build-static -j 8 -C Release -R amount --output-on-failure
Internal ctest changing into directory: C:/Users/hebasto/bitcoin/build-static
Test pro
...

💬 theStack commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2444734571)
> My understanding is that this change is mostly a convenience change for native Windows developers, so that it becomes easier to run binaries after compilation (without installing?).

Even as non-Windows developer, I personally find it quite handy if all binaries end up in a single folder and are not scattered around in different places, unnecessarily still reflecting the in-tree source structure (resulting sometimes in long nested paths, as one can see in the scripted-diff). Turned out to b
...

🚀 fanquake merged a pull request: "cmake: Add `FindZeroMQ` module"
(https://github.com/bitcoin/bitcoin/pull/30903)

💬 petertodd commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2444826150)
On Thu, Oct 24, 2024 at 02:52:55AM -0700, laanwj wrote:
> > CJDNS is still a routed ove rlay network, where packets do not always go the shortest (internet) route to the destination. That is a latency hit vs native IP.
>
> There are some edge cases where super low latency matters, such as mining (or spy nodes :sweat_smile: ), but in general, propagation of anything over the P2P network can be slow and that's fine. There's no tight UI loop. It's not worth to compromise privacy over latency-like
...

💬 ryanofsky commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#issuecomment-2444839496)
> I think there is merit in making the less safe (i.e. `std::string` overload) less convenient to use so that developers who are less familiar with this code don't accidentally use it when they don't have to, but I'll leave a comment on your PR so we can have the conversation there.

I agree with this, and commit b6a39c81e85338bc82f3db924157a599aa7e25fa in this PR shows places where code is doing things like `strprintf("[" + comment + "]")` that get around compile time checking. It'd be great
...

💬 fanquake commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2444839927)
Also added this to `29.x`, as if we are going to change this, it needs to happen along with the CMake switchover, and not be a new breaking change after that.

💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1821213504)
Done.

💬 hebasto commented on pull request "depends: Use `CC_FOR_BUILD` for `config.guess `":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2444855404)
My Guix build:
```
aarch64
a2d14a22db6670a4f33aa4a7f9bd04d45cb861993b51670c4c950bf98f68864f guix-build-707d65ba0d74/output/aarch64-linux-gnu/SHA256SUMS.part
71d45a6c783b287a3369503b34ef08a42b5c4a2c8c2f636cf4665574e4f2f0ac guix-build-707d65ba0d74/output/aarch64-linux-gnu/bitcoin-707d65ba0d74-aarch64-linux-gnu-debug.tar.gz
3f5fb049c4347d25c091ec4412dfb3a9a6488a28aad2cca7bd52d822ff6b5f0f guix-build-707d65ba0d74/output/aarch64-linux-gnu/bitcoin-707d65ba0d74-aarch64-linux-gnu.tar.gz
bb8a6a2a
...

💬 ismaelsadeeq commented on issue "Faster way to get block with prevouts in JSON-RPC":
(https://github.com/bitcoin/bitcoin/issues/30495#issuecomment-2444881418)
I also noticed using `getblock` sequentially on a large number of blocks was slow while checking for clusters of size > 2 in previously mined blocks see https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2113010122.

To investigate further, I conducted a benchmark on a VPS with specs:
- 8 vCPU Cores, 24 GB RAM, 1.2 TB SSD, 32 TB Traffic
- Running Ubuntu 22 with Bitcoin Core on latest master da10e0bab4a3e98868dd663af02c43b1dc8b7f4a

I used a [script](https://gist.github.com/isma
...

📝 ismaelsadeeq opened a pull request: "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC"
(https://github.com/bitcoin/bitcoin/pull/31179)
This PR is motivated by https://github.com/bitcoin/bitcoin/issues/30495#issuecomment-2444881418,
It adds a `reserve` member function to `UniValue` and applies it within the `getblock` RPC to pre-allocate memory, minimizing reallocations. This change provides a significant performance boost, particularly at verbosity levels 1 and 2.


Here are benchmark for this conducted on a VPS with the specs:
- 8 vCPU Cores, 24 GB RAM, 1.2 TB SSD, 32 TB Traffic
- Ubuntu 22 with Bitcoin Core on the lates
...

💬 andrewtoth commented on issue "Faster way to get block with prevouts in JSON-RPC":
(https://github.com/bitcoin/bitcoin/issues/30495#issuecomment-2444907742)
@ismaelsadeeq nice find!

I wonder, could you also benchmark [batch requests](https://www.jsonrpc.org/specification#batch)? Sending a single request that contains `rpcthread` number of `getblock` requests, sequentially and multithreaded on the client side?

💬 andrewtoth commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2444918015)
Concept ACK

💬 achow101 commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2445007388)
light ACK 0f4bc635854597e15ea6968767fc4e5cf5bdd790

💬 achow101 commented on pull request "rpc: getorphantxs follow-up":
(https://github.com/bitcoin/bitcoin/pull/31043#issuecomment-2445022460)
ACK 0ea84bc362f395fd247623c22942eb5ca3d1b874

🤔 theuni reviewed a pull request: "cmake: Add `FindZeroMQ` module"
(https://github.com/bitcoin/bitcoin/pull/30903#pullrequestreview-2402777638)
Post-merge ACK 915640e191b6a17a245f0502bc399d82a6502ccf

🚀 achow101 merged a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110)

🚀 achow101 merged a pull request: "rpc: getorphantxs follow-up"
(https://github.com/bitcoin/bitcoin/pull/31043)