💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r2395761545)
It seems https://github.com/bitcoin/bitcoin/pull/32602/files#diff-1ef3b6a1936b50f3d5ec4a1786d9e2d63d1a3e1815b103e67f20601995f355b4R312-R321 revealed that there's a silent conflict here, we need the [first commit](https://github.com/bitcoin/bitcoin/pull/32313/commits/85979e2015344f05aa60db52b4a76eeadef216ad) from https://github.com/bitcoin/bitcoin/pull/32313 for this to pass after rebase
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r2395761545)
It seems https://github.com/bitcoin/bitcoin/pull/32602/files#diff-1ef3b6a1936b50f3d5ec4a1786d9e2d63d1a3e1815b103e67f20601995f355b4R312-R321 revealed that there's a silent conflict here, we need the [first commit](https://github.com/bitcoin/bitcoin/pull/32313/commits/85979e2015344f05aa60db52b4a76eeadef216ad) from https://github.com/bitcoin/bitcoin/pull/32313 for this to pass after rebase
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408056506)
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 https://github.com/andrewtoth/bitcoin/commit/c256f1b457cbd5b900aa34703eb5853d2449bcde#diff-3eff97d24109f473292b38a6495c5b51e8bda9f4bff9b691cf255968b
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408056506)
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 https://github.com/andrewtoth/bitcoin/commit/c256f1b457cbd5b900aa34703eb5853d2449bcde#diff-3eff97d24109f473292b38a6495c5b51e8bda9f4bff9b691cf255968b
...
💬 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
...
(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
...
(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?
(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.
(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
...
(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
(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).
(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).