💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2480474170)
What your patch demonstrates is that without the fix in this PR (clearing the cache), the invalid block is accepted by the node.

If you then also drop the `assert_equal ... current_block_height)` light, you'll see that the fails at "Block should propagate".

The "Submit again, with the witness" step doesn't replace the invalid block, and `ProcessNewBlock` returns `true` for duplicate blocks and `submitSolution` passes it on.

I think is good, because in Stratum v2 (depending on configurat
...

👍 TheCharlatan approved a pull request: "mining: check witness commitment in submitBlock"
(https://github.com/bitcoin/bitcoin/pull/33745#pullrequestreview-3403009700)
ACK 7d57c4e44451ce1167ba76a73c9cc6f6fd6cfca4

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2480523381)
I have already done that, which parts do you think I'm missing?

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2480567300)
Taken

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2480568701)
`remote_block_before` is set before the two `submitBlock` calls above. I improved the wording a bit and I moved the `getchaintips()[0]["height"]` down to the next test, because that was probably confusing.

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#issuecomment-3471963619)
Pushed [7d57c4e](https://github.com/bitcoin/bitcoin/commit/7d57c4e44451ce1167ba76a73c9cc6f6fd6cfca4) to [2fc95ca](https://github.com/bitcoin/bitcoin/commit/2fc95ca731b52ca419215109c568e89bf683a94b): [compare](https://github.com/bitcoin/bitcoin/compare/7d57c4e44451ce1167ba76a73c9cc6f6fd6cfca4..2fc95ca731b52ca419215109c568e89bf683a94b). No behavior change.

I added a patch the to PR description that reproduces the original issue.

Expanded the `submitSolution` documentation to indicate that a
...

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2480606043)
Added a patch to the PR description which reproduces the issue.

📝 A-Manning opened a pull request: "rest: Query predecessor headers using negative count param"
(https://github.com/bitcoin/bitcoin/pull/33752)
There is currently no mechanism by which to batch-query headers from tip, backwards towards genesis.
This PR extends the REST Blockheaders API to support batch-querying predecessor headers, by providing a negative count parameter.

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

> I do think more improvements could be made here:

These could be followups, so will comment here:

> - documentation could be updated to say what requirements on submitSolution inputs are

Not sure what to write there, basically whatever consensus requires. Previously mining software would the whole coinbase from scratch and we never really documented that.

The Stratum v2 spec does specify what their `SubmitSolution` should look like: https://stratumprotocol.org/
...

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2480649764)
I kept all three, but as I explained in https://github.com/bitcoin/bitcoin/pull/33745#issuecomment-3471963619 two of them are already `false`.

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