💬 instagibbs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1831211399)
I've already made the attempt to completel split out the code paths and it was far more work to understand as I have to use the `BroadcastTransaction` interface which is quite different than `testmempoolaccept`'s requirements.

This is the most minimal fix I can devise:
```
diff --git a/src/validation.cpp b/src/validation.cpp
index 3e48335255..c89050e895 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1790,10 +1790,16 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackag
...

💬 mzumsande commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1831240767)
Thanks, changed to that.

💬 hebasto commented on pull request "cmake: Improve robustness and usability":
(https://github.com/bitcoin/bitcoin/pull/31233#issuecomment-2460090395)
My Guix build:
```
aarch64
b4bd41285ec597b50489a3e3bf8ffde5e557d514ad5860fa3cfdd04da2144839 guix-build-4b6a842c2801/output/aarch64-linux-gnu/SHA256SUMS.part
3222cf16fe93c3df73efc360150b384bf968f18ce929875bf384c9833e6411e7 guix-build-4b6a842c2801/output/aarch64-linux-gnu/bitcoin-4b6a842c2801-aarch64-linux-gnu-debug.tar.gz
68cb1589be0fe45c8d9bd2dfa96b2610d49fcec5621b678a952db857493fdae9 guix-build-4b6a842c2801/output/aarch64-linux-gnu/bitcoin-4b6a842c2801-aarch64-linux-gnu.tar.gz
9e289ec7
...

💬 fanquake commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2460093756)
Tested on Arch, which is one of the few distros that ship CMake config files for `libevent-dev`. You could add this diff:
```diff
diff --git a/doc/build-unix.md b/doc/build-unix.md
index a5ad4df11d..4f04b4fd9f 100644
--- a/doc/build-unix.md
+++ b/doc/build-unix.md
@@ -182,7 +182,7 @@ Setup and Build Example: Arch Linux
-----------------------------------
This example lists the steps necessary to setup and build a command line only distribution of the latest changes on Arch Linux:


...

💬 hebasto commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2460106051)
> Tested on Arch, which is one of the few distros that ship CMake config files for `libevent-dev`. You could add this diff:
>
> ```diff
> diff --git a/doc/build-unix.md b/doc/build-unix.md
> index a5ad4df11d..4f04b4fd9f 100644
> --- a/doc/build-unix.md
> +++ b/doc/build-unix.md
> @@ -182,7 +182,7 @@ Setup and Build Example: Arch Linux
> -----------------------------------
> This example lists the steps necessary to setup and build a command line only distribution of the latest change
...

💬 instagibbs commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1831296233)
Would this be correct? I would like this to be easily verifiable, and filling in the FIXMEs would make that simple
```
diff --git a/src/test/fuzz/p2p_headers_presync.cpp b/src/test/fuzz/p2p_headers_presync.cpp
index 08e8d08c08..41ac8d172b 100644
--- a/src/test/fuzz/p2p_headers_presync.cpp
+++ b/src/test/fuzz/p2p_headers_presync.cpp
@@ -92,12 +92,14 @@ CBlockHeader ConsumeHeader(FuzzedDataProvider& fuzzed_data_provider, const uint2
header.nNonce = 0;
// Either use the previous d
...

💬 theuni commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#discussion_r1831297585)
Yes. To be clear, I really don't like the arbitrary nature of this at all. I was hoping someone would have a better idea for an approach to selection.

I really meant this as an RFC because I agree that what's here is a pretty crappy solution. But it's clear that pch is a win, so I think we should come up with _something_.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1831358176)
This is a great idea!

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1831368185)
I think `self` is no longer a good name for this parameter if we make this static. It should just be `pair`.

Same for `SetDirty` and `SetFresh`.

👍 andrewtoth approved a pull request: "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906#pullrequestreview-2418862806)
re-crACK aa2f3139529c054b011a0f75ff314e6d63f0d977

It might be worth updating the `AddFlags` comment now, but otherwise non-blocking nits.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1831367497)
nit: this comment is no longer accurate after d5e3bbf440aa948df8159999fd4eb5275c354b93

We should just remove the part about a self reference, and only mention the sentinel.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1831374417)
nit: `// Check that we can clear state then re-set it`

💬 glozow commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#discussion_r1831397906)
What do you mean any value in?

💬 hebasto commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2460305663)
My Guix build:
```
aarch64
b526b46943f39780cb2a204cd64159f7dd4f502e482256ba78371c82d717a591 guix-build-5a96767e3f53/output/aarch64-linux-gnu/SHA256SUMS.part
77714a2034b96c572598342da2147a638abd626b29e4ccecfe786f53dbf72a4e guix-build-5a96767e3f53/output/aarch64-linux-gnu/bitcoin-5a96767e3f53-aarch64-linux-gnu-debug.tar.gz
a32ce8d85863ec00f75b37ab7b10bfc37e85efd3a1016ed66ce6cbd0024d018a guix-build-5a96767e3f53/output/aarch64-linux-gnu/bitcoin-5a96767e3f53-aarch64-linux-gnu.tar.gz
751eba9b
...

💬 instagibbs commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#discussion_r1831404449)
would the suggested change potentially give coverage

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: Implement changeset interface for mempool"
(https://github.com/bitcoin/bitcoin/pull/31122#pullrequestreview-2413495210)
ACK 9512bd34a3d74d15c3ce1086a80a1edfa5ac3acf
With some comments.

💬 ismaelsadeeq commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1828023652)
Can just be vector of `TxHandle` here.

💬 ismaelsadeeq commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1831210600)
The return here is redundant.

💬 ismaelsadeeq commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1827993798)
For clarity:

```suggestion
// Finally, check that we can prioritize tx_child_1 to get package1 into the mempool.
```

We can also prioritize tx_parent_1 to get package1 in, but the nFeeDelta must be high enough to evict package3.

💬 ismaelsadeeq commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1831172493)
This seems fine to remove because it's redundant anyways.
But worth noting in the commit message.