📝 l0rinc opened a pull request: "coins: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906)
Similarly to https://github.com/bitcoin/bitcoin/pull/30849, this cleanup is intended to de-risk https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1739909068 by simplifying the coin cache public interface.

`CCoinsCacheEntry` provided general access to its internal flags state, even though, in reality, it could only be `clean`, `fresh`, `dirty`, or `fresh|dirty` (in the follow-up, we will remove `fresh` without `dirty`).

Once it was marked as `dirty`, we couldn’t set the state back t
...

💬 l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1760050345)
Split out to https://github.com/bitcoin/bitcoin/pull/30906

🤔 andrewtoth reviewed a pull request: "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906#pullrequestreview-2305366216)
Concept ACK

Can we remove any other tests that are trying to add flags that are not FRESH or DIRTY and so are now useless?

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760052862)
Why was this changed?

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760052629)
This assertion is obviated now.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760053494)
`But` can be removed now.

💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2351617058)
My graph on a log scale, looks similar to sipa's:


Results txt: [2024-09-14-bench.txt](https://github.com/user-attachments/files/17006092/2024-09-14-bench.txt)

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2351617378)
> I could reproduce the macOS 2h timeout issue on GHA yesterday. However, today I also fail to reproduce them.

Actually, it looks like the GHA macOS CI timeouts remain today, for example: https://github.com/bitcoin/bitcoin/actions/runs/10833854937/job/30163805645?pr=30866 or https://github.com/bitcoin/bitcoin/actions/runs/10829942238/job/30163952659?pr=30856

I am not really familiar with GHA, nor with macOS, so it would be good if someone else checked:

* Does the macOS 13 GHA task succ
...

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760072626)
Because class declarations don't need semicolons, but I'll revert if you find it distracting

💬 furszy commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1760070775)
Isn't it a bit aggressive to execute the provided predicate every 0.05 seconds for the entire duration?
Beyond flooding the logs, the predicate likely calls an RPC command that locks `cs_main` or some other important mutex, which slows down the node.

For instance, the `feature_assumeutxo.py` one, will call `getpeerinfo()` 60 times.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760102028)
Because class declarations don't need semicolons, but I'll revert if you find it distracting

💬 fjahr commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1760114937)
> I guess fs::is_empty(opts.blocks_dir) could be replaced by not fs::exists("blk0000.dat")?

I think that alone wouldn't work if the user upgrades a pruned node to v28?

Anyway, I was going to revisit this today but I simply don't see the failures anymore currently 🤷‍♂️ I will get back to it if I see them occur again.

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1760138462)
I simply took the same interval as `wait_until` uses. If the slowdown was an issue it should be much more apparent from the use there, so I did a simple benchmark and increasing the sleep from `0.05` to `0.2` lead to a considerable slowdown of the test suite overall (8min to 10:45min), so I don't think this would be an issue here either.

We could still lower the interval. Since we generally don't expect the predicate to become false it shouldn't hurt performance as much as changing it in `wai
...

💬 naiyoma commented on pull request "p2p: Increase inbound capacity for block-relay only connections":
(https://github.com/bitcoin/bitcoin/pull/28463#issuecomment-2351678725)
Concept ACK to increase the number of maximum connections to **200**, enabling an increase in outgoing block relay connections later on.

💬 vostrnad commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2351700413)
I've benchmarked commit e624a9bef16b6335fd119c10698352b59bf2930a, both in this branch and cherry-picked onto a few selected commits (fresh sync to block 400,000 from a local node, using default settings, all binaries self-built using Mingw-w64, around 10 runs for each version).

Summary: some slowdown due to #28052 definitely remains, but it's under 5%.

|version|result|note|
|-|-|-|
|fa7f7ac040a9467c307b20e77dc47c87d7377ded|2112 ± 17 seconds|First commit of #28052, and last commit before
...

📝 pablomartin4btc opened a pull request: "Fix both ipv6 proxy setup and reachable networks display issues in Options Dialog (UI only, no functionality impact)"
(https://github.com/bitcoin-core/gui/pull/836)
Currently, setting up a proxy (whether SOCKS5 or Tor) with an IPv6 address works correctly via the command line or configuration file in both `bitcoind` and `bitcoin-qt` (also from the UI the ipv6 address gets saved properly in `settings.json`). However, the UI does not reflect this properly, which can create confusion. Since some ISPs and VPNs still experience issues with IPv6, users may mistakenly think there is a problem with Bitcoin Core, when in fact the proxy setup is functioning as expect
...

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760258646)
Replaced it with a flag validation

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1760258689)
Done

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2351740285)
> Can we remove any other tests that are trying to add flags that are not FRESH or DIRTY and so are now useless?

I've merged `NO_ENTRY` and `char(0)` values - this simplifies the cases somewhat.
Do you think we can remove any of the lines? There aren't any duplicates, even after the above change...

🤔 glozow reviewed a pull request: "cluster mempool: optimized candidate search"
(https://github.com/bitcoin/bitcoin/pull/30286#pullrequestreview-2305615852)
reACK 9ad2fe7e69e, just have a question about the docs