💬 maflcko commented on pull request "netif: fix compilation warning in QueryDefaultGatewayImpl()":
(https://github.com/bitcoin/bitcoin/pull/34093#issuecomment-3670881044)
> I am not sure why there is no "comparison of integers of different signs" on Linux:
>
> ```c++
> #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
> (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
> (nlh)->nlmsg_len <= (len))
> ```
>
> We pass `int64_t` for `len`. The first comparison checks it against `(int)sizeof(...`, but the 3rd comparison checks it against `nlmsg_len` which is `__u32`, so no matter what t
...

💬 Bicaru20 commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#issuecomment-3670886818)
ACK https://github.com/bitcoin/bitcoin/commit/1841bf9cb67bca99273b3efa38633113422f5882

💬 hodlinator commented on pull request "miniscript refactor: Remove unique_ptr-indirection":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3670894292)
Rebased to solve trivial conflict with #33192.

💬 billymcbip commented on pull request "test: Improve code coverage for pubkey checks":
(https://github.com/bitcoin/bitcoin/pull/34099#issuecomment-3670937312)
@rkrux perhaps my force push on the other PR broke the code coverage job.

By the way, I've also verified that the branches are called by adding `assert(false)` to those branches. `script_tests` then failed as expected.

💬 theuni commented on pull request "Add initial vectorized chacha20 implementation for 2-3x speedup":
(https://github.com/bitcoin/bitcoin/pull/34083#issuecomment-3670950989)
> Could we get some comments in the code as to compiler versions (and architecture) that failed to be efficient enough with lambdas, to hopefully avoid future people modernizing the code without checking that these problems have gone away? Presumably will be a long time before we can modernize to `template for` (which doesn't even seem to be on [cppref's compiler support page](https://en.cppreference.com/w/cpp/compiler_support.html) yet?)...
>
> (Only thing that I do think is "that bad" about
...

🚀 fanquake merged a pull request: "[30.x] Finalise v30.1"
(https://github.com/bitcoin/bitcoin/pull/34092)

💬 anuragchvn-blip commented on pull request "doc: Use multipath descriptors in descriptors.md and linked test":
(https://github.com/bitcoin/bitcoin/pull/34100#issuecomment-3670966160)
@rkrux Thank you for the review and testing!

I've pushed a fix addressing both issues:

1. **Test failure**: Added `"range": 1000` parameter to `importdescriptors` to enable change address generation. The multipath descriptor now properly expands to both receive and change paths.

2. **Documentation clarity**: Changed `(0)` and `(1)` to `(/0)` and `(/1)` notation as suggested - much clearer in context.

The test should now pass. Could you re-test when you have a chance? Thanks again fo
...

💬 ismaelsadeeq commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#issuecomment-3670973117)
The rebase is trivial it seems https://github.com/bitcoin/bitcoin/compare/master...ismaelsadeeq:bitcoin:12-2025-threadpool

💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2631659231)
I agree that it would be nice, but it's not so simple in my opinion:
`ConnectTip()` being protected or not seems to be the least of problems:
1. It would need to become `virtual`, because it is called internally by `ABCStep`. `ConnectTip()` is a member of `Chainstate`, not `ChainstateManager`.
2. even if `ConnectTip` was virtual, it won't help with the current way to cast to a derived type (`auto& chainman = static_cast<TestChainstateManager&>(*g_setup->m_node.chainman);`) - which seems to be
...

🤔 marcofleon reviewed a pull request: "refactor: enable `readability-container-contains` clang-tidy rule"
(https://github.com/bitcoin/bitcoin/pull/34095#pullrequestreview-3593752904)
code review ACK a8a0a0ff77309a95645f6de00595c1dc39cd6eff

Tested with boost 1.74, good to go.

💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2631690440)
No, I think you are right here. There probably isn't a way (even when using UB) to mock this without touching the validation module.

👍 pablomartin4btc approved a pull request: "refactor: enable `readability-container-contains` clang-tidy rule"
(https://github.com/bitcoin/bitcoin/pull/34095#pullrequestreview-3593810848)
cr-ACK a8a0a0ff77309a95645f6de00595c1dc39cd6eff

Verified the revert of `boost::multi_index::contains` instances (2).

💬 l0rinc commented on pull request "miniscript refactor: Remove unique_ptr-indirection":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3671068636)
Rebased locally, soft reset against re-rebased PR HEAD is empty.

ACK 7909f9f7d6d780f0cf2a1e94267b71157122b0f5

💬 fjahr commented on pull request "Prevent file descriptor exhaustion from too many RPC calls":
(https://github.com/bitcoin/bitcoin/pull/27731#issuecomment-3671141568)
Closing since it becomes increasingly unlikely that we will see a 2.2 release or that we might want to upgrade to it even when it comes. Additional context: https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2025-12-18#1180379;

✅ fjahr closed a pull request: "Prevent file descriptor exhaustion from too many RPC calls"
(https://github.com/bitcoin/bitcoin/pull/27731)

🤔 janb84 reviewed a pull request: "refactor: enable `readability-container-contains` clang-tidy rule"
(https://github.com/bitcoin/bitcoin/pull/34095#pullrequestreview-3593897430)
cr ACK a8a0a0ff77309a95645f6de00595c1dc39cd6eff

Also confirmed that clang-tidy (still) works

💬 pinheadmz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3671216043)
push to 625088468a3f2f1438fd71d1b87210e16904fa58:

- Fix conflicts with master, mostly `LogDebug()` conflicts from #29641
- Silent conflict with a test added in #33657 since the new URL parsing is more tolerant

📝 fanquake opened a pull request: "doc: add missing copyright headers"
(https://github.com/bitcoin/bitcoin/pull/34106)
Takes care of some queries from #34084.

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2631870905)
This needs to be here because of the linter which checks that `std::filesystem` isn't used outside of this `fs` namespace.

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2631874918)
I don't think `txns_randomized` gets reordered between runs, its randomness depends on the order that inserts/deletes happen in the mempool which is deterministic. I've also checked that this harness is fully deterministic with the `deterministic-fuzz-coverage` script.

💬 stringintech commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2631875686)
For reviewers:

As discussed offline, adding `ArgsManager` and the `G_TRANSLATION_FUN` definition that should be added because of it, is causing different compile/link issues since the symbol is also defined in the kernel library and @sedited is suggesting to have the symbol removed from there.

So adapting `bitcoin-chainstate.cpp` to use the `ArgsManager` could be done in a follow-up.