💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1765031560)
```suggestion
inline void SetClean() noexcept
```

👍 instagibbs approved a pull request: "fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work"
(https://github.com/bitcoin/bitcoin/pull/30918#pullrequestreview-2312643576)
ACK 7b85fd1443488afe60235e4c33bd956cb9c8d562

Thanks for the quick follow-up on suggestion! Makes the test significantly easier to read.

💬 instagibbs commented on pull request "fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work":
(https://github.com/bitcoin/bitcoin/pull/30918#discussion_r1765029395)
IIUC this would be a conservative overestimate, since the compact block and block messages don't move `base` forward?

If so, probably should note that here.

🤔 hodlinator requested changes to a pull request: "test: generalize HasReason and use it in FailFmtWithError"
(https://github.com/bitcoin/bitcoin/pull/30921#pullrequestreview-2312632081)
Concept ACK

💬 hodlinator commented on pull request "test: generalize HasReason and use it in FailFmtWithError":
(https://github.com/bitcoin/bitcoin/pull/30921#discussion_r1765022377)
Unless `HasReason` gets a new ctor that takes non-ref `string` which we can `std::move` into (and it can `move` from internally into the member), we are now allocating extra `string` copies on every `FailFmtWithError`-call.

I prefer going back to `string_view` here.

Yet another possible alternative is to construct `HasReason` copies outside and pass them in. Nice to memory bad slightly ugly using `HasReason` outside of the Boost macro.
```diff
diff --git a/src/test/util_string_tests.cpp
...

💬 hodlinator commented on pull request "test: generalize HasReason and use it in FailFmtWithError":
(https://github.com/bitcoin/bitcoin/pull/30921#discussion_r1765024172)
nit: Curly-maximalism.
```suggestion
BOOST_CHECK_EXCEPTION(util::ConstevalFormatString<WrongNumArgs>::Detail_CheckNumFormatSpecifiers(wrong_fmt), const char*, HasReason{error});
```

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1765039768)
Do we need a new function if we're only calling it the one time here? Just seems like unnecessary misdirection.

💬 l0rinc commented on pull request "test: generalize HasReason and use it in FailFmtWithError":
(https://github.com/bitcoin/bitcoin/pull/30921#discussion_r1765059519)
Nice, pushed!

💬 l0rinc commented on pull request "test: generalize HasReason and use it in FailFmtWithError":
(https://github.com/bitcoin/bitcoin/pull/30921#discussion_r1765059960)
Not needed after your previous suggestion.

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2358480289)
@glozow could we get a unit test for that specific behavior change :pray:

@marcofleon I presume it's simply things that "shouldn't happen":
0) Parent `MempoolRejectedTx`, put in reconsiderable filter
1) Child`MempoolRejectedTx`, put in orphanage
2) Child `MempoolRejectedTx` for other reason, now in reject filter
3) Parent `ReceivedTx`, found in reconsiderable filter, fetches package with fully rejected child

I'd feel better if we also simply not return a package we should never consid
...

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1765046861)
I think these checks on down can be pulled into `CheckPackageToValidate`?

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

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1765076974)
Converting flags to enums isn't the responsibility of this method, that's why I've extracted it.

💬 hodlinator commented on pull request "test: generalize HasReason and use it in FailFmtWithError":
(https://github.com/bitcoin/bitcoin/pull/30921#discussion_r1765097890)
Avoid extra allocations (from my [original diff](https://github.com/bitcoin/bitcoin/pull/30921#discussion_r1765022377)):
```suggestion
inline void FailFmtWithError(std::string_view wrong_fmt, const HasReason& error)
```

💬 1440000bytes commented on issue "Use IPv4-encoded IPv6 address to get IPv4 node address with port number from DNS seeds":
(https://github.com/bitcoin/bitcoin/issues/30900#issuecomment-2358536436)
> @1440000bytes As you can see by clicking through to the removal pull-req, the problem wasn't "insufficient nodes", it was failing to implement filtering properly: https://github.com/bitcoin/bitcoin/issues/29911

Sharing the PR rationale/description below with people who ACKed it:

```
This seeder no longer appears to be serving sufficient addresses.

Fixes https://github.com/bitcoin/bitcoin/issues/29911
```

```
ACKs for top commit:
1440000bytes:
ACK https://github.com/bitco
...

💬 l0rinc commented on pull request "test: generalize HasReason and use it in FailFmtWithError":
(https://github.com/bitcoin/bitcoin/pull/30921#discussion_r1765109845)
Seems I'm not that sensitive to these things for tests :)
Done, thanks

💬 i-am-yuvi commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2358601668)
tested ACK 🔥

Successfully ran demo scripts and functional tests!!

I have been using these tracepoints for 3-4 months from now and I find it very useful. They help you keep track of your peers, also I am working on a tool that detects spy peers and for that these tracepoints are very useful.

Great work @0xB10C.

⚠️ maflcko opened an issue: "Intermittent failure in p2p_1p1c_network.py", line 58, in raise_network_minfee assert_greater_than(node.getmempoolinfo()['mempoolminfee'], FEERATE_1SAT_VB) ; AssertionError: 0.00001000 <= 0.00001000"
(https://github.com/bitcoin/bitcoin/issues/30922)
https://github.com/bitcoin/bitcoin/actions/runs/10923649833/job/30320807014?pr=30921#step:7:46110

```
node0 2024-09-18T14:10:27.733922Z [httpworker.2] [src/rpc/request.cpp:232] [parse] [rpc] ThreadRPCServer method=getmempoolinfo user=__cookie__
test 2024-09-18T14:10:27.734000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/Users/runner/work/bitcoin/bitcoin/test/functional/test_fr
...

💬 maflcko commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2358652413)
The CI failed, but the logs are missing :(

https://github.com/bitcoin/bitcoin/pull/29664/checks?check_run_id=30286710796

```
�[0m�[0;31mp2p_ibd_stalling.py --v2transport | ✖ Failed | 2465 s
```

I'll re-run 10 times.

💬 maflcko commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2358671416)
On CI:

```
�[0m�[0;31mtool_utxo_to_sqlite.py | ✖ Failed | 2 s