💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761348362)
?
```Suggestion
assert((depgraph.Descendants(child) & children) == SetType::Singleton(child));
```

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761406415)
nit
```Suggestion
// Compute the ancestors of parents that are not already ancestors of child.
```

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761301820)
commit 77e07eba11391868f83db22bf2280e02b5a8cd78

add a motivation for these additions in commit message?

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761676550)
I don't see where these are being deleted? The only removal below is removing things that aren't in `Positions()`, which the newly added transactions should be? Likely misreading this.

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761630898)
in commit message for 7510a65127db610df4be9cdd95eaef0c23b3e2ff

"This commits starts introducing support" are we expecting more support later?

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761348880)
?
```Suggestion
assert((depgraph.Ancestors(parent) & parents) == SetType::Singleton(parent));
```

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1763548308)
Ergonomically speaking, I expect this to be used to remove a set of "direct conflicts" and all descendants of that set, so this wouldn't cause invalid views of the underlying cluster's graph relationships.

Is this correct?

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1763550167)
do we want to leave a note that now-unused entries not at the end can be non-empty/dirty? If that's not possible, a comment explaining that would be helpful too.

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761452737)
could use this in AddDependency as a one-liner?
```
AddDependencies(SetType::Singleton(parent), child);
```

📝 marcofleon opened a pull request: "fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work"
(https://github.com/bitcoin/bitcoin/pull/30918)
A followup to https://github.com/bitcoin/bitcoin/pull/30661

The added assertion just makes sure that the fuzz test is working as intended. If we're sure that the total work of the test chain is never more than minimum chain work, then we can be sure that the later assertion failure would actually mean that a bug in the headers presync logic was found.

💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2356502855)
> Another follow up improvement I'd like is direct assertion that the "chainwork" of the header chain never exceed minimum chainwork, or if that is exceeded, we can drop the assertion that the chain never extends past genesis block.

Followup here: https://github.com/bitcoin/bitcoin/pull/30918

Also, I was mistaken in this [comment](https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1750661750). The test as is already does build a chain that is more than 16 headers long. So, there was
...

💬 jonatack commented on pull request "doc: remove Eclipser fuzzing documentation":
(https://github.com/bitcoin/bitcoin/pull/30908#issuecomment-2356532534)
> I think we could even remove eclipser from the docs entirely. As you noted as well, it looks unmaintained and I don't think any of us are actively using it.

Done, thanks everyone for the feedback. Leaving for a follow-up the addition of a concolic testing tool example with bug.

💬 maflcko commented on pull request "doc: remove Eclipser fuzzing documentation":
(https://github.com/bitcoin/bitcoin/pull/30908#issuecomment-2356545824)
review ACK 735436df8cebf28d4bcf0ad7e8f1fd7f19191001

👍 brunoerg approved a pull request: "doc: remove Eclipser fuzzing documentation"
(https://github.com/bitcoin/bitcoin/pull/30908#pullrequestreview-2310438395)
ACK 735436df8cebf28d4bcf0ad7e8f1fd7f19191001

💬 marcofleon commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1763652769)
Why is it that we don't initialize these with the`Txid` and `Wtxid` types? Have been thinking about making `RelayTransaction` type safe, so just wondering.

👍 theuni approved a pull request: "ci: Use `ninja` to build in macOS native CI job"
(https://github.com/bitcoin/bitcoin/pull/30915#pullrequestreview-2310455658)
ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f.

I was going to suggest this as well :)

💬 jonatack commented on pull request "ci: Use `ninja` to build in macOS native CI job":
(https://github.com/bitcoin/bitcoin/pull/30915#issuecomment-2356604457)
Concept ACK in using https://ninja-build.org / https://github.com/ninja-build/ninja for a CI task as a point of comparison.

🤔 jonatack reviewed a pull request: "ci: Use `ninja` to build in macOS native CI job"
(https://github.com/bitcoin/bitcoin/pull/30915#pullrequestreview-2310534703)
LGTM but am new to CMake builds.

ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f

💬 davidgumberg commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763724027)
Could be scripted diff:

```bash
sed -i -E 's/CheckWriteCoins\(([^,]+), ([^,]+), ([^,]+), ([^,]+), ([^,]+), ([^,]+)\);/CheckWriteCoins({\1, \4}, {\2, \5}, {\3, \6});/' src/test/coins_tests.cpp
```

👍 hebasto approved a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2310572569)
re-ACK 807a429a33cd9b1986f1ebb8fd761887f556afd8, rebased only since my recent [review](https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2300357639).

My Guix build:
```
aarch64
f6a91156bc9ac2ec9367d75a433d903db4fe5c4ff0c5680289882f1ca58b9759 guix-build-807a429a33cd/output/aarch64-linux-gnu/SHA256SUMS.part
2704a2fbc76888fb9073631513ea1fc2203edb344814dbe5db02fd51c6d9949d guix-build-807a429a33cd/output/aarch64-linux-gnu/bitcoin-807a429a33cd-aarch64-linux-gnu-debug.tar.gz
a5c
...