💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408379260)
> Sure, so I tried updating #31132 to use the thread pool here. It makes the change simpler if we were to already have this thread pool merged. However, the threads write to non-synchronized shared vectors, which the main thread will read. We need to make sure the write happens before the read. I initially stored the futures and wait on them like this [andrewtoth@c256f1b#diff-3eff97d24109f473292b38a6495c5b51e8bda9f4bff9b691cf255968ba80d85dR151-R164](https://github.com/andrewtoth/bitcoin/commit/c
...

💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2414239068)
nice! I like the clearer variable names. used this diff in f284834 and added you as a coauthor.

1 difference with using `pindex` instead of `to_mark_failed` in [the last couple of lines](https://github.com/bitcoin/bitcoin/pull/32950/commits/f284834170d625b86de05d7b98d91eb8c39ab55e#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98L3777) - (ex: `InvalidChainFound(to_mark_failed);`) is that:
- `to_mark_failed` used to track the last disconnected block index
- `pindex` is t
...

💬 yancyribbens commented on pull request "docs: add doc comment for SRD selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2430345487)
Some of the other algorithms use that wording, for example see coin-grinder: https://github.com/bitcoin/bitcoin/pull/33622/files#diff-d473ed8396f9451afb848923cfcfaa630c9811a78e07f3ae1ffd3a65da218accR208. Therefore just copying a pattern that already exists. Do you not like that wording in coin-grinder also?

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2447930605)
After cluster mempool, mini_miner should be modified anyway (and I think this whole MiniMinerMempoolEntry object will disappear, see for example my draft [here](https://github.com/bitcoin/bitcoin/commit/d24f02699ec4af215a4ffae3a2ae2457f71bf93f#diff-5dfb364874cfa42dd98492c3c3c1b07aafbe73ef7ecd895246d2fea00499fd6c)). So I don't think it's worth re-implementing this interface in this PR.

💬 maflcko commented on issue "GetSerializeSize's return type should not be platform dependent":
(https://github.com/bitcoin/bitcoin/issues/33709#issuecomment-3457434288)
> ... LLM-generated. I'm not sure how to go about that. I am not comfortable with the idea of an LLM making changes to consensus-critical code. On the other hand, if the code is correct just closing the PR seems inappropriate.

I looked at the PR, and while it appears "correct", it is randomly only changing `size_t` to `uint64_t` in half the places, even if they are on adjacent lines (https://github.com/bitcoin/bitcoin/pull/33712/files#diff-a9bf14cec48b76a03a785de76b8bc71c5db17d494c7f02c29dfde26
...

💬 0xB10C commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#issuecomment-3473985400)
Note that the @plebhash comments were actually remove from the archive (as it just pulls the GitHub API. Gone from the GitHub API means gone from the latest version of the archive and mirroring) and might thus be missing from the mirroring in the next rebuild..

See https://github.com/bitcoin-data/github-metadata-backup-bitcoin-bitcoin/commit/9ca6ff3e35249f8ccb710de7cee6ff13e028f0ee#diff-a4bfd29817b2ed302dbaf2c9bd4514ace8aa1ffeacf02046400d93b526b8ab02L847

💬 roconnor-blockstream commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3476339242)
@TheCharlatan I touch on this in the above, but I do want to explicitly note that the order of public keys that Counterparty uses changed in response to #5247 and the [the keys used to be in the other order](https://github.com/CounterpartyXCP/counterparty-core/commit/f1a8c82914b2f913560496e3d1becc1b63f88118#diff-b9154ac64e110b5561c5bcadc77b532aaea3f44eb53e9c4a76ae2ed3c6fad8b8L231-R261).

💬 TheBlueMatt commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3478062912)
I was misled by https://github.com/bitcointranscripts/bitcointranscripts/pull/593/files#diff-e25cced7d45bde5a991f8ac638162335638ea1ba39b21a5cb76d5e5e31b31345R9-R10

💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3478064586)
> I was misled by https://github.com/bitcointranscripts/bitcointranscripts/pull/593/files#diff-e25cced7d45bde5a991f8ac638162335638ea1ba39b21a5cb76d5e5e31b31345R9-R10

Ok, I see, that could have been more clear as either "depends on `rpki-client`" (the only dependency outside of python) or "depends on RPKI, IRR and Routeviews downloads" or something. I will clarify in the PR.

💬 TheCharlatan commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2486129322)
I'm observing a memory leak in this fuzz test similar to the one we had for the thread pool. Over there, we disabled logging and instantiate only a single pool instance: https://github.com/bitcoin/bitcoin/pull/33689/files#diff-68602d972fe2b027e3987eff0042c27f3f00fc161b7fe871bdb147571f348298R49-R58 . Maybe similar things can be done here?

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2486365567)
Generally yes, but here I wanted to copy the [original code](https://github.com/bitcoin/bitcoin/pull/33637/files#diff-2e16a7ac99a65c1617112c3c326fba499b0010fe6c48b92b59bbc9696d883585L104) as closely as possible

💬 stickies-v commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2487174964)
http://github.com/bitcoin/bitcoin/pull/32604/commits/d541409a64c60d127ff912dad9dea949d45dbd8c#diff-44fd50b51e8fc6799d38f193237fb921ec9d34306c448f64837524a17bac06eeR471-R472 introduced prefixing all messages with `[*]` to indicate that ratelimiting is currently being applied.

💬 l0rinc commented on pull request "util: Allow Assert() in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2498620712)
I'm also not a fan of globally disabling these, they could hide [real problems](https://github.com/bitcoin/bitcoin/pull/32313/files#diff-bca63b54ad7fcc2df12f3237fd8775810c1b849c49110f702920655778fcc248).

Doing a `git grep -n "const auto&.*Assert("` shows there are only a few of these
```
src/index/coinstatsindex.cpp:204: const auto& tx_undo{Assert(block.undo_data)->vtxundo.at(i - 1)};
src/init.cpp:2006: const auto& tip{*Assert(chainman.ActiveTip())};
src/node/miner.c
...

💬 151henry151 commented on pull request "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3503646384)
> Sorry, but one more rebase is needed.

I think I've rebased correctly here -- I removed an additional `SKIP_BUILD_RPATH OFF` override that seemed like it would no longer be needed, https://github.com/bitcoin/bitcoin/commit/a02282d20c87471a3399e4061c7edad7ecdb391f#diff-2a07660db8ebdbbf967cf42f6b4da3ffc8558a322f45a3a08ce292c9e91767b8L13

Guix build for a02282d20c87:
a3647a46e00124b207263b695d9598cf9fe6c845ea9072c933462dd0e31a0ed9 guix-build-a02282d20c87/output/aarch64-linux-gnu/SHA256SUMS
...

🤔 hebasto reviewed a pull request: "refactor: Add missing include in bitcoinkernel_wrapper.h"
(https://github.com/bitcoin/bitcoin/pull/33825#pullrequestreview-3438418410)
ACK fa1e8d8bad92f5fba2b086d78581df4c8123b098.

[Here](https://github.com/hebasto/bitcoin/commit/11bc5cca61a9edad38d070bd5046355fcf58c7ee#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2) is IWYU's diff based on https://github.com/bitcoin/bitcoin/pull/33810:
```diff
--- a/src/kernel/bitcoinkernel_wrapper.h
+++ b/src/kernel/bitcoinkernel_wrapper.h
@@ -5,12 +5,19 @@
#ifndef BITCOIN_KERNEL_BITCOINKERNEL_WRAPPER_H
#define BITCOIN_KERNEL_BITCOINKERNEL_WRAPPER_H

-#include <kern
...

💬 l0rinc commented on pull request "scripted-diff: Remove obsolete comment":
(https://github.com/bitcoin/bitcoin/pull/33826#issuecomment-3506770544)
code review ACK 36724205fc1f226d7b5493ed0212c336e7f2ae84

Looks like this is a leftover from [before the cmake migration](https://github.com/bitcoin/bitcoin/commit/6ce50fd9d0ae6850d54bf883e7a7c1bcb6912c5c#diff-7533d479b9e3ad56dcfbf6daaa21317373e8233da5ea13714554115c66057aa2L22-L132).
Checked locally, other Python 3 references seem unrelated.

💬 maflcko commented on pull request "refactor: Add missing include in bitcoinkernel_wrapper.h":
(https://github.com/bitcoin/bitcoin/pull/33825#issuecomment-3511768897)
> [Here](https://github.com/hebasto/bitcoin/commit/11bc5cca61a9edad38d070bd5046355fcf58c7ee#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2) is IWYU's diff based on #33810:

I can't find that in the CI output. I don't think iwyu runs on stand-alone headers without a cpp file?

💬 hebasto commented on pull request "refactor: Add missing include in bitcoinkernel_wrapper.h":
(https://github.com/bitcoin/bitcoin/pull/33825#issuecomment-3511843037)
> > [Here](https://github.com/hebasto/bitcoin/commit/11bc5cca61a9edad38d070bd5046355fcf58c7ee#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2) is IWYU's diff based on #33810:
>
> I can't find that in the CI output. I don't think iwyu runs on stand-alone headers without a cpp file?

It's here: https://github.com/hebasto/bitcoin/actions/runs/19194191546/job/54872770781:
```
<snip>
2025-11-08T14:21:35.1873049Z (/home/runner/work/_temp/src/kernel/bitcoinkernel_wrapper.h
...

💬 l0rinc commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#issuecomment-3521439654)
> Since when is this "unused"?

Did some digging, https://github.com/bitcoin/bitcoin/commit/1f0e7ca09c9d7c5787c218156fa5096a1bdf2ea8#diff-706988c23877f8a557484053887f932b2cafb3b5998b50497ce7ff8118ac85a3L135-L165 inlined `ComputeMerkleRoot` in one case, leaving the `proot` and `pmutated` values unused in `MerkleComputation` (cc: @sipa, @Sjors).
And `BlockWitnessMerkleRoot` was introduced in https://github.com/bitcoin/bitcoin/commit/8b49040854be2e26b66366aeae1cba4716f93d93 where it was already
...

💬 rkrux commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2527400928)
In d1a2e278572f70d0d68a21312b2cc7de27f326df "test: replace class ValidWitnessMalleatedTx with function"

Unlike the `malleate_tx_to_invalid_witness` function that is quite generic (https://github.com/bitcoin/bitcoin/pull/33793/files#diff-1698fabd4ddd298eb28e234d97f34e744f568852b77361384c8edb38819de5e3R259), this function at the moment doesn't seem generic enough to be present in an util file. Few points regarding the function I noticed:

1. The name doesn't reflect the implementation, the fu
...