💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3253662102)
Waiting for indexing to complete, did a 1min perf dump:
<img width="1684" height="1177" alt="image" src="https://github.com/user-attachments/assets/b3da4ca7-57d1-4737-849a-f0ad1216f306" />

* 35.2% of the time is spent in the `CTransaction` constructor, eagerly computing hashes which `LocationsIndex` doesn't even use.
* 11.7% is spent in the `GetSerializeSize` calls inside `LocationsIndex::CustomAppend()`.

Would be nice for the `LocationsIndex` if hashes were lazily computed (avoided) and
...

💬 vasild commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2322112177)
This ends up creating real sockets and generates non-loopback network traffic. Fixed in https://github.com/bitcoin/bitcoin/pull/31349 in commit 005b8ca02c `test: avoid non-loopback network traffic from node_init_tests/init_test`.

💬 m3dwards commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2322125897)
Runner size wasn't extensively tested for performance, your logic for going for smaller runners seems sound to me.

💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3253719585)
Rebased d35ceaeb463bc836ac4fc4bd6dd4f387647f33fb -> daf0e9a3d45f42889fc5895fc580c73d060d2711 ([blocktreestore_4](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_4) -> [blocktreestore_5](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/blocktreestore_4..blocktreestore_5))

* Fixed conflict with #33274

💬 m3dwards commented on pull request "CI: silent merge check":
(https://github.com/bitcoin/bitcoin/pull/33145#issuecomment-3253719746)
Currently working on an alternative approach of one job that loops through PRs, might close this PR in favour of the other one based on feedback.

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-3253741259)
Rebased 5537ac00983a9abd24689411b5d76058d7a02f1b -> b27be6cb18dde53863d83d2f2f61d4c92916257b ([spendblock_11](https://github.com/TheCharlatan/bitcoin/tree/spendblock_11) -> [spendblock_12](https://github.com/TheCharlatan/bitcoin/tree/spendblock_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_11..spendblock_12))

⚠️ fanquake opened an issue: "ci: single Windows cache takes up > 25% of total cache space"
(https://github.com/bitcoin/bitcoin/issues/33305)
See here https://github.com/bitcoin/bitcoin/actions/caches?query=sort%3Asize-desc:
```bash
win64-native-vcpkg-binary-d91a6ca2e3cf6a687ef72004b964264224a060b043eec2cf156395fb74cf67d7
# 2.6 GB cached yesterday
```

A single Windows vcpkg cache is 2.6GB. Our total cache storage limit is 10.0GB. It'd be good if a single Windows cache wasn't taking up 25% of our total allocated cache. Maybe the vcpkg build can be modified, to improve this.

💬 hebasto commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3253787672)
I've tested this in my personal repo and can confirm the re-run job [behaviour](https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3185092065) described by @janb84, which is also documented [here](https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs).

In my opinion, the current behavior is useful for reproducing issues in the CI.

However, before merging a PR, it's much easier to re-run the CI to check for merge conflicts than to rebase the
...

💬 ryanofsky commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2322207556)
In commit "ci: detect outbound internet traffic generated while running tests" (f400e0bb82920adf50fec1b9d701e8e85e62030a)

Thanks for the fix! I think instead of manipulating CreateSock for this specific test it would be probably better to add `gArgs.ForceSetArg("-natpmp", "0");` to `BasicTestingSetup::BasicTestingSetup`. Reasons:

- It would work generally for all tests, not just this one test
- This would make unit tests more consistent with [functional tests](https://github.com/bitcoin/b
...

👍 ryanofsky approved a pull request: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349#pullrequestreview-3185369594)
Code review ACK a9ac49c8accffeffaae72f54021fd939c951844c

Thanks for the fix! Note that this fix may (I'm not sure) mask an integer overflow issue https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2294091366 that seems like it is a real bug.

The new tcpdump mechanism to detect unexpected traffic is pretty simple and seems very nice. Another idea could be to have unit test setup use `CreateSock` to throw errors if code attempts a remote connection to make errors a little easier to ca
...

📝 fjahr opened a pull request: "index: Force database compaction in coinstatsindex"
(https://github.com/bitcoin/bitcoin/pull/33306)
This PR forces full database compaction on coinstatsindex every 10,000 blocks. This does not seem to come with any considerable performance impact, though I did not test on many different systems.

Motivation: Currently, coinstatsindex ends up using a lot of 55kb files if this is not the case. This is likely related to very fast writes occurring during sync but so far I could not pin down where exactly the difference in behavior is coming from. It seems to be a LevelDB internal thing though, s
...

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3253868966)
> Is my understanding correct that AFL will run init (create the static dir), then fork into several fuzz processes, which run for N iterations and then shut down and delete the static dir? (On the next re-fork, it will fail to find the static dir)?

My description was a bit vague. Since the harness uses `-testdatadir` in init to create the static datadir, it does not get deleted in the TestingSetup destructor (by design of `-testdatadir`). Any time init is called, it will wipe the static data
...

💬 ryanofsky commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-3253880125)
Do we know if this change might fix the integer overflow error reported https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2294091366?

💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2322297692)
re: https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2294123857

> But maybe there is a real bug on this line if it is subtracting `20 - 28`:

This might be fixed by #32159

✅ fanquake closed an issue: "ci: single Windows cache takes up > 25% of total cache space"
(https://github.com/bitcoin/bitcoin/issues/33305)

💬 fanquake commented on issue "ci: single Windows cache takes up > 25% of total cache space":
(https://github.com/bitcoin/bitcoin/issues/33305#issuecomment-3253902831)
This was modifed, but only for master (#32182), so the cache entries are still generated from any release branch, and bloat the global cache. Apparently this may be non-trivial to backport, so can just be left as-is, for now.

💬 purpleKarrot commented on pull request "cmake: Inherit WERROR setting for secp256k1 build":
(https://github.com/bitcoin/bitcoin/pull/33297#issuecomment-3253905515)
> Here is some historical context.

Don't take it personal. You did a great job migrating from autotools to CMake. I have zero doubts about that.

💬 wrapperband commented on issue "Please restrict Data Carrier/OP Return to < 80 bytes please before releasing 3":
(https://github.com/bitcoin/bitcoin/issues/33298#issuecomment-3253925320)
Please do not excessively increase op_return size. The coder will be as responsible for miss use as tornado cash developer - and they ended up in prison.

💬 maflcko commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3253938081)
> documented

thx, added to description

> In my opinion, the current behavior is useful for reproducing issues in the CI.

If there is a hypothetical issue to reproduce by re-running, which is no longer present with current master, it seems best to reproduce with an exact commit outside this repo. (Also, I can't really remember a single instance where this was ever needed)

🤔 rkrux reviewed a pull request: "doc: update multisig tutorial to use multipath descriptors"
(https://github.com/bitcoin/bitcoin/pull/33286#pullrequestreview-3185548980)
Concept ACK 226836e70501abf9ac2434900e74eec31edd716e

As mentioned in this comment https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3248584420 as well, I think all these commits can be squashed into one.