💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101000637)
Added as comment.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100993968)
I have changed the variable names to `max_cluster_count` and `max_cluster_size` everywhere.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100995536)
`GetTotalTxSize` is a nice color.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100996101)
Done, but also removed in a later commit.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100995144)
Do you mean in the TxGraph::AddTransaction definition in txgraph.h? I have expanded the comments in the fuzz test around this a bit.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100993169)
I have expended the commit message of both Trim commits.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101001371)
I've added a comment to highlight that.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101002887)
Done.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101002139)
Good idea, I went with `OVERSIZED_SINGLETON`.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101006466)
Instead of this big unintuitive comment, I have replaced it with `/*oversized_tx=*/m_quality == QualityLevel::OVERSIZED_SINGLETON`. With that, I don't think the `Assume` is neccesary?

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101012572)
I have expanded the commit messages.

💬 laanwj commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2101043927)
fwiw: `GetACP` is a windows system function implemented in `kernel32.dll`, defined in the `winnls.h` header. There shouldn't be a mingw-specific definition.

`CP_UTF8` is also defined in that header and has the following value:
```c++
#define CP_UTF8 65001
```

`1252` is codepage [windows-1252](https://en.wikipedia.org/wiki/Windows-1252), a legacy encoding.

💬 polespinasa commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2899113774)
> when I open regtest after a few days...

Maybe we should force this code to work only in mainnet? It doesn't make sense the time considerations for regtest. Maybe testnet and signet could still use it.

👍 willcl-ark approved a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32292#pullrequestreview-2859072889)
crACK a0d1f69b555fa0c76df1a63a0b127c7816596107

Checked all patches were backported cleanly and appear in the release notes.

Did not build/test.

🤔 Crypt-iQ reviewed a pull request: "fuzz: doc: add info about `afl-system-config` for macOS"
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2859098136)
ACK 61ea5f3

I noticed that a couple lines above, the link to "selecting the best AFL compiler..." is invalid and has instead moved to https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/fuzzing_in_depth.md#a-selecting-the-best-afl-compiler-for-instrumenting-the-target. I can open a PR to fix the doc link.

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2101176481)
Fixed

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2101196912)
Does :+1: mean "That is what I meant" or "It's fixed now"?

💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2899294093)
@laanwj @ryanofsky

Thank you for your feedback!

I've restored the `shell` option in the `subprocess` API, and employed it on non-Windows systems.

💬 sipa commented on pull request "allocators: Apply manual ASan poisoning to `PoolResource`":
(https://github.com/bitcoin/bitcoin/pull/32581#issuecomment-2899298395)
Concept ACK

💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2899323908)
> Then, given that the fallback seems to be needed by OpenBSD and newlibc (possibly others), it may be better to keep it?

It would be even better to use `ftruncate` on openbsd. But yes, simply assuming `posix_fallocate` availability is going to fail on some platforms.