💬 ryanofsky commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069550613)
> The correct fix is to just add the includes iwyu is asking for.

I think this is about cases where IWYU on one platform asks you to add (or remove) an include, and IWYU on other platform asks you to remove (or add) the same include. A reasonable way to resolve these is to decide which IWYU is correct and add pragmas to silence the other IWYU. Other ways to resolve it could be to define mappings, or submit fixes upstream, or choose a single platform to be your canonical IWYU platform and igno
...

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069564494)
Yes, the keep pragma seems useful, but I personally wouldn't recommend using the no_include pragma in this context

💬 Sjors commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3069566904)
Better to point to #6722 as it has the discussion from back then.

💬 stratospher commented on pull request "test: fix intermittent failure in rpc_invalidateblock.py":
(https://github.com/bitcoin/bitcoin/pull/32968#discussion_r2204928129)
yeah makes sense, I guess this was the reason some of the test was moved up in https://github.com/bitcoin/bitcoin/pull/30479#issuecomment-2892651335

🤔 instagibbs reviewed a pull request: "cluster mempool: add TxGraph work controls"
(https://github.com/bitcoin/bitcoin/pull/32263#pullrequestreview-3016273688)
reviewed via `git range-diff master 18e40b6cd5a11e23b7a0654bdd12f08e610ce980 4788c87f7c119f35c551da05e7c43bd5be51420c`

💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204933624)
It will not "expend" iterations on oversized levels, so it can return "true" when doing no work.

💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204876202)
s/quality/QualityLevel/

💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204911282)
The new logic means that you may not have enough to make it considered `ACCEPTABLE` BUT you may end up with `OPTIMAL` anyways and are able to continue until you hit a difficult cluster. In practice this may end up with a lot more clusters known as optimal when their topology is simple.

Might be worth a comment on the "if (!improved) return false;" line

💬 maflcko commented on issue "ci: don't pass GIT_EXEC_PATH inside test container":
(https://github.com/bitcoin/bitcoin/issues/32935#issuecomment-3069728608)
Maybe an easy fix could be to not pass any env vars, except those with a key specified in `ci/test/00_setup_env.sh`?

🤔 instagibbs reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-3016494838)
reviewed through 790f6e7a72d31988c5be9f2d520a04d0c0edfa09

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205033216)
6ce95bffee0f809d9bf4d8fed6f8a7df077fa83f

I think txorphanage_sim subsumes the older `txorphan` harness now unless I'm missing something

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205019742)
do we want to call this after every `AddTx` above instead of after adding all additional announcers?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205060872)
```Suggestion
assert(m_orphans.size() >= m_unique_orphans);
assert(m_orphans.size() <= m_peer_orphanage_info.size() * m_unique_orphans);
```

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205053496)
d5097d6eef85eab971352cf78fb94a30c6e6f127

looks like you forgot to assert the calculated set matches anything ala

assert(m_peer_orphanage_info == reconstructed_peer_info);

💬 stringintech commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#issuecomment-3069805059)
[1154618](https://github.com/bitcoin/bitcoin/commit/11546183c70def6c0aa539642fd1c9ada3d46840) to [61e800e](https://github.com/bitcoin/bitcoin/commit/61e800e75cffa256ccdbc2ffc7a1739c00880ce0) - Applied suggestion by @stratospher to move helper methods before their respective subtest. Thanks!

💬 stringintech commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2205078542)
While I personally prefer a top-down approach where the main test methods are visible at the top without much scrolling, I am seeing the pattern you described is mostly respected across the tests, so I applied it.

💬 maflcko commented on pull request "test: fix intermittent failure in rpc_invalidateblock.py":
(https://github.com/bitcoin/bitcoin/pull/32968#discussion_r2205112609)

“Since header doesn't have block data, it can't be chain tip” -> “Since the header doesn't have block data, it can't be the chain tip” [missing definite articles]

💬 maflcko commented on pull request "test: fix intermittent failure in rpc_invalidateblock.py":
(https://github.com/bitcoin/bitcoin/pull/32968#issuecomment-3069863625)
lgtm ACK 28416f367a5d720d89214aa8ef94013caa1d1a40

Can be tested via by moving the lines after ` self.nodes[0].reconsiderblock(block.hash)` down again and observing a failure.

👋 ishaanam's pull request is ready for review: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896)

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#issuecomment-3069883620)
This PR is now ready for review.