💬 fanquake commented on pull request "kernel: revert accidentally removed copyright header":
(https://github.com/bitcoin/bitcoin/pull/34105#issuecomment-3670801358)
ACK 85314dc0bf871226c0e43446bb79f49630d15f4a

🚀 fanquake merged a pull request: "kernel: revert accidentally removed copyright header"
(https://github.com/bitcoin/bitcoin/pull/34105)

💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2631512181)
I was assuming that directly calling `ActivateBestChain` wouldn't work in a fuzz test due to certain things not being mocked out, but I wonder if it's possible to instead call it and not have to re-implement it here?

💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3670814276)
reACK https://github.com/bitcoin/bitcoin/commit/db2d39f642979f929261e5f1cd67f0c2f2ca045f

💬 theuni commented on pull request "Add initial vectorized chacha20 implementation for 2-3x speedup":
(https://github.com/bitcoin/bitcoin/pull/34083#discussion_r2631524729)
@l0rinc As the code is written at the moment, all platforms must opt-in to vectorization as opposed to opting out. I'm not sure that's the best approach, but I figured that was a reasonable starting point.

My reasoning for that was: consider non-x86, non-arm+neon platforms. For the most part, I'm assuming they're under-powered. Enabling (for example) 4x blocks/sec for mipsel would probably cause a drastic slowdown. Obviously there are lots of other powerful platforms, but I figured those woul
...

🚀 fanquake merged a pull request: "fuzz: Add fuzz target for block index tree and related validation events"
(https://github.com/bitcoin/bitcoin/pull/31533)

💬 sipa commented on issue "p2p: `seed.bitcoin.sipa.be` is down":
(https://github.com/bitcoin/bitcoin/issues/34104#issuecomment-3670853633)
I don't see anything wrong with it, and `check-dnsseeds.py` works for me.

✅ fanquake closed an issue: "p2p: `seed.bitcoin.sipa.be` is down"
(https://github.com/bitcoin/bitcoin/issues/34104)

💬 fanquake commented on issue "p2p: `seed.bitcoin.sipa.be` is down":
(https://github.com/bitcoin/bitcoin/issues/34104#issuecomment-3670863688)
Could be a local issue.

💬 joshdoman commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2631558465)
Hmm, I'm open to removing the word "efficient." I included it because `PrecomputedTransactionData` is strictly unnecessary for non-taproot input verification, but it does make verification faster over multiple inputs.

That may be self-explanatory. If so, we can definitely change the description to `Create precomputed transaction data for script verification.`

👍 ismaelsadeeq approved a pull request: "http: replace WorkQueue and single threads handling for ThreadPool"
(https://github.com/bitcoin/bitcoin/pull/33689#pullrequestreview-3593444540)
Code review ACK 2de0ce5cd85e1b99e318883964df318ffb615fe4 👾

💬 ismaelsadeeq commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2631432438)


In "util: introduce general purpose thread pool”
258518d880e81938fcd9672b37bc640e8a06c930

This comment is incorrect because this check alone is not sufficient to guarantee that `Stop()` is never called from a worker thread.

Consider the following scenario: `Stop()` is called both from a worker thread and from a non-worker thread sequentially. The call from outside the pool may execute first (unless the caller explicitly waits on the future from the thread pool `Stop()`). The non thread pool
...

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