💬 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

💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2358711183)
https://github.com/bitcoin/bitcoin/actions/runs/10923053194/job/30318787539?pr=30849#step:6:2225

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1765239421)
thanks! Updated in [a9964c0](https://github.com/bitcoin/bitcoin/pull/30875/commits/a9964c04447745435747d9cc557165c43902783b)

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1765239532)
thanks! Updated in [a9964c0](https://github.com/bitcoin/bitcoin/pull/30875/commits/a9964c04447745435747d9cc557165c43902783b)

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1765239674)
thanks! Updated in [a9964c0](https://github.com/bitcoin/bitcoin/pull/30875/commits/a9964c04447745435747d9cc557165c43902783b)

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2358734956)
> ACK [bc4a929](https://github.com/bitcoin/bitcoin/commit/bc4a929cd716cd2b412c70754749d4fda0ca2a10)
>
> pico-nit: your branch messes up the 80 column wrapping in a few spots in `doc/developer-notes.md`. This is not really consistent through the docs anyways, so I don't mind personally.

thanks for the suggestion! But I think I'll leave those for now since they don't relate to the autotools to cmake update too much and I would prefer to keep the PR more focused on those changes

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2358736153)
ACK a9964c04447745435747d9cc557165c43902783b