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

💬 davidgumberg commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763730500)
Although I guess this section would be ugly to do as a scripted diff

💬 hebasto commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#discussion_r1763730646)
@theuni

You might be interesting in reviewing this PR, as this comment was added at your request while working on the CMake staging branch, if I recall correctly :)

💬 davidgumberg commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763731654)
Nit: Could be made into a loop as below

🤔 instagibbs reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2310464388)
it's likely the wrong place for the coverage, but I added some coverage for iterative additional and removal of transactions. Take what you think is useful if any:

```
diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp
index be06898a6e..62b5ec2343 100644
--- a/src/test/fuzz/cluster_linearize.cpp
+++ b/src/test/fuzz/cluster_linearize.cpp
@@ -1,19 +1,20 @@
// Copyright (c) The Bitcoin Core developers
// Distributed under the MIT software license, see
...

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1763665495)
Was reconstructing graphs incorrectly leading to inconsistent sets. Better to fail here early.

```Suggestion
{
// We can't add relationships to holes
Assume(parents.IsSubsetOf(m_used));
```
a check like this for `AddDependency` would also be good:

`Assume(SetType::Singleton(parent).IsSubsetOf(m_used));`

💬 davidgumberg commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2356666728)
Concept ACK on dropping the bitfield interface for `CCoinsCacheEntry` flags

🤔 theuni reviewed a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2310630029)
Concept ACK.

Though, `obj/` was acting as a bit of a namespace here (IIRC, that was another reason for it to be in a subdir). Especially since this is included with `<>` as opposed to `""`, while we're at it, I think we should rename this file. `build.h` seems generic enough to collide with something else. `bitcoin-build.h` maybe?

💬 pablomartin4btc commented on pull request "cli: Improve error message on multiwallet cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2356746568)
Updates:
- Removed validation commit which I'll added in a separate PR as [suggested](https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2354091658) by @jonatack.
- Updated description and title accordingly with above.
- Added test checking -rpcwallet=<filename> is not doable as [suggested](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1762109275) by @jonatack.
- Replaced `-rpcwallet=<filename>` which is incorrect with `-rpcwallet=<walletname>` as suggested by @mzumsande
...