💬 glozow commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#issuecomment-2112437062)
reACK 9365baa489

💬 stickies-v commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1601589963)
Oops no you're absolutely right, I didn't see the for-loop.

💬 maflcko commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1601592294)
Not sure I understand this change either. This seems to be mixing the third alternative in the pull request description with the approach you wanted to take?

> Defeats the purpose of a RAII wrapper for FILE* which automatically closes the file when it goes out of scope and there are a lot of users of AutoFile.

💬 stickies-v commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2112454690)
re-ACK 27af3910e5fa9915d4913b82443db216d1b1d61b

👍 cbergqvist approved a pull request: "Support JSON-RPC 2.0 when requested by client"
(https://github.com/bitcoin/bitcoin/pull/27101#pullrequestreview-2057910491)
re ACK cbc6c440e3811d342fa570713702900b3e3e75b9

All functional tests passed (with a few automatic skips), except `feature_dbcrash` - slow, unrelated => excluded, and `feature_index_prune` => timed out because rebase with bumped timeout has been held-off.

💬 cbergqvist commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1601581136)
Would have gone the opposite way and called it `throw_errors` since it is an.. exception.. to maintain legacy behavior. Sorry for not catching that earlier.

💬 sr-gi commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1601615548)
Oh my bad, I was thinking about children that may have multiple missing parents, not only the conflicting one.

💬 sr-gi commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1601615800)
Yep, my bad, nvm

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1601658971)
Covered in [55f16f5](https://github.com/bitcoin/bitcoin/pull/30065/commits/55f16f56f4abeae1f5560cc616ea0b9b31a88073)

💬 t-bast commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2112590380)
While I wouldn't trust myself to correctly code-review this PR, I'd be happy to work on e2e tests that would leverage this for lightning channels fee-bumping (based on [eclair](https://github.com/acinq/eclair)) if it can help validate the logic and get this PR merged.

I'd like to highlight again how important this feature is for lightning (and probably for many other L2 protocols on top of bitcoin today). This is the critical step that allows us to mitigate pinning of a commitment transaction
...

👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2058024841)
Code review ACK 27af3910e5fa9915d4913b82443db216d1b1d61b

💬 ryanofsky commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1601650396)
In commit "[[refactor]] Check CTxMemPool options in constructor" (27af3910e5fa9915d4913b82443db216d1b1d61b)

I think it would be clearer to avoid using && and std::move, since using these doesn't actually make things more efficient. In 27af3910e5fa9915d4913b82443db216d1b1d61b, one copy and one move of the option struct will happen. The copy happens when calling the CTxMemPool constructor (since the parameter type is a value parameter), the move happens when returning from SetOptionLimits.

B
...

💬 theuni commented on pull request "crypto: disable asan for sha256_sse4 with clang and -O0":
(https://github.com/bitcoin/bitcoin/pull/30097#issuecomment-2112603762)
> Could you like to that thread from the comment added to the source, then this is probably good to go.

Done

✅ willcl-ark closed a pull request: "Use shared_ptr for CNode inside CConnman"
(https://github.com/bitcoin/bitcoin/pull/28222)

💬 willcl-ark commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#issuecomment-2112616409)
Closing for now

🚀 ryanofsky merged a pull request: "p2p: index TxOrphanage by wtxid, allow entries with same txid"
(https://github.com/bitcoin/bitcoin/pull/30000)

💬 ryanofsky commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2112626070)
I merged this as-is since it has so many acks, but it probably makes sense to follow up on the last few comments and suggestions.

💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1601722461)
I was just re-using the pattern used for the `ChainstateManager` construction, but I like this more. Taking.

💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2112657721)
Thanks for the reviews, going to take @ryanofsky's suggestion, hope it is not much work to review again

Updated 27af3910e5fa9915d4913b82443db216d1b1d61b -> 33f2a708e392edb2501f555efef34a558da9d717 ([mempoolArgs_8](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_8) -> [mempoolArgs_9](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/mempoolArgs_8..mempoolArgs_9))
* Addressed @ryanofsky's [comment](https://github.com
...

📝 glozow opened a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111)
See #27463 for full project tracking.

This contains the first 4 commits of #30110, which require some thinking about thread safety in review.
- Introduce a new `m_tx_download_mutex` which guards the transaction download data structures including `m_txrequest`, the rolling bloom filters, and `m_orphanage`. Later this should become the mutex guarding `TxDownloadManager`.
- `m_txrequest` doesn't need to be guarded using `cs_main` anymore
- `m_recent_confirmed_transactions` doesn't need it
...

💬 hebasto commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601750695)
This commented line maybe requires more explanation?