💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2480669020)
If you apply the patch in the description and also drop the `sync_all` call, you'll see that the second node see the block as `headers-only`.

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#issuecomment-3472081330)
cc @ajtowns

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2480717926)
I think this was a copy-paste thing from elsewhere in the code (this "dummy" comment appears at line 111 as well, and seems to have been introduced when this fuzz test was first created) -- I presume the origin is just that the MemPoolRemovalReason doesn't matter for how `removeRecursive()` behaves, it's just passed on to callbacks.

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2480723944)
Incorporated in 0097138011b6

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2480726400)
Done in d00d8a8a84e43cb79e81482a2a85532209dd9ab2

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2480727366)
Done in d00d8a8a84e43cb79e81482a2a85532209dd9ab2

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2480729561)
Done in 74654eaf1bef5b578b97111af20b3de39c39cec6

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2480730805)
Done in 74654eaf1bef5b578b97111af20b3de39c39cec6

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2480733194)
Done in 0097138011b6e502bfbae528304a6a7e6b6462ad

💬 fanquake commented on pull request "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3472140702)
> Guix build for 1e6aa74b4b628e8acafe6a3d2aaf2a6aa0d9d290:

Thanks. However you should do a build for all `HOSTS`. As this is changing code that effects all `HOSTS`.

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3472188061)
I've addressed most of the review feedback, other than the mutation testing notes from @brunoerg which I still need to review (thank you for the analysis!), and some documentation items that were brought up [here](https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440825043) and [here](https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440904975).

I'll work on incorporating the documentation improvements in #33591.

👍 dergoegge approved a pull request: "fuzz: refactor memcpy to std::ranges::copy to work around ubsan warn"
(https://github.com/bitcoin/bitcoin/pull/33743#pullrequestreview-3403414040)
utACK fa4b52bd16189d40761c5976b8427e30779aba23

🤔 stickies-v reviewed a pull request: "validation: ensure assumevalid is always used during reindex"
(https://github.com/bitcoin/bitcoin/pull/31615#pullrequestreview-3403430467)
Concept ACK: reindex validation behaviour being different to IBD validation behaviour is undesirable and should be avoided.

Slight approach NACK: I think this is probably the smallest diff we can get to improve reindex behaviour, but I think there are drawbacks that outweigh the benefits:
1) it still doesn't make IBD and reindex validation behaviour the same: with the current approach blocks that are NOT part of the assumevalid chain will still be validated as assumevalid. While it doesn't s
...

✅ fanquake closed an issue: "fuzz: connman fuzz target: runtime error: null pointer passed as argument 2, which is declared to never be null"
(https://github.com/bitcoin/bitcoin/issues/33643)

🚀 fanquake merged a pull request: "fuzz: refactor memcpy to std::ranges::copy to work around ubsan warn"
(https://github.com/bitcoin/bitcoin/pull/33743)

💬 fanquake commented on pull request "ci: Add missing python3-dev package for riscv64":
(https://github.com/bitcoin/bitcoin/pull/33746#issuecomment-3472257325)
ACK facf8b771a192ba17c8c4f9c43f248d83b3c8015

🚀 fanquake merged a pull request: "ci: Add missing python3-dev package for riscv64"
(https://github.com/bitcoin/bitcoin/pull/33746)

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2481027571)
I wanted to use the SQL [order-by](https://www.postgresql.org/docs/current/queries-order.html#QUERIES-ORDER) terminology here
> Ascending order puts smaller values first, where “smaller” is defined in terms of the < operator.
Similarly, descending order is determined with the > operator.

But you're right, I inverted the terminology, added a dedicated test to check for hard-coded order (first param is increasing, second is decreasing), updated the comments (this is why I find comments a lazy
...

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2481027700)
I strongly dislike code comments, I prefer explaining with live code over dead comments - and if people disagree they can always do a blame which instantly reveals the purpose since I also over-explain in commit messages usually.
Are the names `old_comparator` in a `cblockindex_comparator_equivalence ` not enough to make it obvious that? I
I have added a comment anyway, let me know if it helps.

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2481027916)
I look at fuzz tests as exploratory tools that are hard to write and run and debug and do code coverage on - not trivial to maintain (see https://github.com/bitcoin/bitcoin/issues/33731), it's definitely not my go-to way to test something. This is just a randomized property based unit test - easy to debug, easy to run and understand.
But looking at the tests again, I can make it more deterministic so that it hits all important branches which allows me to reduce the iteration count - code covera
...