👍 fanquake approved a pull request: "ci: Remove redundant RUN_UNIT_TESTS_SEQUENTIAL"
(https://github.com/bitcoin/bitcoin/pull/33136#pullrequestreview-3185106147)
ACK fae610d8581a1a0624b57fe0c2595c9695d677c8

💬 fanquake commented on issue "ci: GHA fallback centos task runs out of space":
(https://github.com/bitcoin/bitcoin/issues/33293#issuecomment-3253562126)
> Yes we can try to free up space. I'll take a look where the space is being used and see what we can free.

#33304 should free up ~1.5GB of disk usage, an take some pressure off caching generally.

🚀 fanquake merged a pull request: "ci: Remove redundant RUN_UNIT_TESTS_SEQUENTIAL"
(https://github.com/bitcoin/bitcoin/pull/33136)

💬 hebasto commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3253566957)
> The PR changes "re-run" default behaviour. Forcing it to use the new state instead of running the old state again.

I suggest making this clearer in the PR description.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3253651928)
`f400e0bb82...9dcbf6ccde`: fix a new case of non-loopback traffic from `node_init_tests/init_test`.

This PR originally contained a few fixes of tests that generated network traffic + a CI change to catch such future cases in CI. Then the tests fixes were moved to https://github.com/bitcoin/bitcoin/pull/31646 and merged. Then the activity here waned.

Now there is a new case of network traffic generated by a test which went in `master` unnoticed. I have fixed that and included it here.

`9
...

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