💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2112374433)
Re https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2057592650

> nit: some IWYU fixes, mostly because of the newly introduced bilingual_str and Assert usage:

A bunch of these were broken before this PR, so I wasn't sure if they make sense to repair here. For test files I stopped asking for repairing them when reviewing other PRs, it just feels too Sisyphean if they are not consistently checked by other reviewers anyway. I wish we'd start enforcing this properly though.

💬 cbergqvist commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1601574911)
nit: `JSONErrorReply` -> `JSONRPCErrorReply`, although it could be argued that it actually does write a JSON object in the response.

💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2112426141)
Thank you for the review @stickies-v,

Updated dacdb7962c5fef8db26f6fa31facb606165d1d1e -> 27af3910e5fa9915d4913b82443db216d1b1d61b ([mempoolArgs_7](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_7) -> [mempoolArgs_8](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_8), [compare](https://github.com/TheCharlatan/bitcoin/compare/mempoolArgs_7..mempoolArgs_8))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1601457942), checking s
...

💬 glozow commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2112430969)
reACK 186a00e644 via range-diff

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