💬 feelancer21 commented on issue "Cannot open assumeUTXO snapshot":
(https://github.com/bitcoin/bitcoin/issues/31643#issuecomment-2585843653)
Thank you. I did not read the error message correctly.

💬 pablomartin4btc commented on issue "Remove libevent as a dependency (HTTP / cli / torcontrol)":
(https://github.com/bitcoin/bitcoin/issues/31194#issuecomment-2585885945)
I've checked that and I see where the problem is...

If you run the following it works correctly (using/ changing `src/test/httpserver_tests.cpp`):

```
uri = "/rest/endpoint/someresource.json?p1=v1%20&p1=v2";
BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1 ");
```
```
./build/src/test/test_bitcoin --log_level=all --run_test=httpserver_tests
...
./test/httpserver_tests.cpp(52): info: check GetQueryParameterFromUri(uri.c_str(), "p1").value() == "v1 "
...

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1912521917)
> I think the URI validation was performed before checking the params, so the exception was raised and caught by the test?

It seems that the libevent parser doesn't differentiate if the URI is invalid whether the query syntax is wrong (missing the '?') or there's an invalid char within the URI (`%` not followed by 2 hexa chars - [RFC 3986](https://datatracker.ietf.org/doc/html/rfc3986#section-2.1) and libevent only interprets it if the encoded chars are withing the variable's value, [not in t
...

📝 l0rinc opened a pull request: "optimization: increase default LevelDB write batch size to 64 MiB"
(https://github.com/bitcoin/bitcoin/pull/31645)
The UTXO set has grown significantly recently, and flushing it from memory to LevelDB often takes over 20 minutes after a successful IBD with large dbcache values.
The final UTXO set is written to disk in batches, which LevelDB sorts into SST files.
By increasing the default batch size, we can reduce overhead from repeated compaction cycles, minimize constant overhead per batch, and achieve more sequential writes.

Experiments with different batch sizes (loaded via assumeutxo at block 840k,
...

💬 l0rinc commented on pull request "optimization: increase default LevelDB write batch size to 64 MiB":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-2585900399)
Visual representation of the measurements (16MiB was the previous default, 64MiB is the proposed one):

<img width="780" alt="image" src="https://github.com/user-attachments/assets/93ab3fff-385b-4093-8016-ab2a251cf92a" />

💬 kilrau commented on pull request "BIP-322 basic support":
(https://github.com/bitcoin/bitcoin/pull/24058#issuecomment-2585912510)
Same question @kallewoof , why did you close this? It's a much needed feature.

💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1912840515)
Testnet3, testnet4 and mainnet all have a difficulty 1 genesis block, so it wouldn't make a difference.

💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1912853526)
Fixed

💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1912853717)
I changed it to "to 2"

💬 Sjors commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2586547919)
@hodlinator a Western Digital spinning disk. Much slower than SSD but fine for just writing out blocks to disk.

(I'm assuming we're not doing something foolish like writing the block, then reading it again to do the xor and writing it back)

@l0rinc I wiped the blocks, indexes and chainstate dirs between runs. Both times I mention exclude the 8 minute flush, which remained similar.

I agree with @maflcko that a difference of 10 minutes is probably not statistically significant. I don't ha
...

💬 vasild commented on pull request "ci: change the build to be verbose by default":
(https://github.com/bitcoin/bitcoin/pull/31619#issuecomment-2586556645)
> How is this different from just looking at the `C++ compiler flags ....................` output?

The exact compilation command contains more information than just the flags. I guess this is why the second build (after a failure) is verbose. There is also this:

```
NOTE: The summary above may not exactly match the final applied build flags
if any additional CMAKE_* or environment variables have been modified.
To see the exact flags applied, build with the --verbose option.
...

💬 Sjors commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2586587671)
Ok, so now we have `bitcoin-util` and `bitcoin-chainstate` in `libexec`, with `bitcoin-gui` and `bitcoin-node` back in `bin` - which can be moved by #31375. That seems fine to me.

When I do `sudo cmake --install build` on macOS 15.2 it puts the binaries in `/usr/local/bin` and `/usr/local/lib` (not `/usr/local/libexec` or `/usr/libexec`). Is that intended?

📝 vasild opened a pull request: "test: avoid internet traffic"
(https://github.com/bitcoin/bitcoin/pull/31646)
Avoid generating outbound traffic on a non-loopback interface during tests. Fix all tests, including ones that generate DNS traffic.

---

This is a subset of https://github.com/bitcoin/bitcoin/pull/31349 containing only the changes to the tests, without the CI changes to detect future regressions.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2586591525)
Extracted the changes to the tests in https://github.com/bitcoin/bitcoin/pull/31646 (suggested by @fjahr and @maflcko).

👍 hodlinator approved a pull request: "doc: Improve dependencies documentation"
(https://github.com/bitcoin/bitcoin/pull/31634#pullrequestreview-2546121330)
re-ACK 1fcc1ce0edcc8ae522ab9103ad79adf58fc0fe0e

Thanks for applying my suggestions.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2586611555)
@fanquake, is the problem you reported above https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2573295321 when the tests run on the host, without a docker?

💬 l0rinc commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2586629442)
@Sjors, your setup is already extremely fast - it seems this optimization shines mostly on commodity hardware, which I assume is used more often.
Let's wait for other reproducers.

💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2586643113)
> When I do `sudo cmake --install build` on macOS 15.2 it puts the binaries in `/usr/local/bin` and `/usr/local/lib` (not `/usr/local/libexec` or `/usr/libexec`). Is that intended?

Target output locations and their installation paths are independent properties. This PR modifies the former only. I'll look into modifying the latter accordingly.

💬 fanquake commented on pull request "doc: Improve dependencies documentation":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1912951874)
Python isn't required.

👍 dergoegge approved a pull request: "p2p: track and use all potential peers for orphan resolution"
(https://github.com/bitcoin/bitcoin/pull/31397#pullrequestreview-2546241855)
Code review ACK 86d7135e36efd39781cf4c969011df99f0cbb69d

Other reviewers already mentioned the nits that I would have had as well. I think they are fine to address in a follow up.