📝 Sjors opened a pull request: "test: have miner_tests use Mining interface and fewer cs_main locks"
(https://github.com/bitcoin/bitcoin/pull/31581)
Needed for both #31283 and #31564.
By using the Mining interface in `miner_tests.cpp` we increase its coverage in unit tests.
We mainly call the `createNewBlock()` method, but also `getTip()->height` and `submitSolution()`. Calls to the latter are alternated with direct calls to `Chainman`'s `ProcessNewBlock`, because e.g. `net_processing.cpp` uses that.
#31564 proposes an alternative to `TestBlockValidity` which requires that `cs_main` is _not_ held. Prepare for this by not holding `cs
...
(https://github.com/bitcoin/bitcoin/pull/31581)
Needed for both #31283 and #31564.
By using the Mining interface in `miner_tests.cpp` we increase its coverage in unit tests.
We mainly call the `createNewBlock()` method, but also `getTip()->height` and `submitSolution()`. Calls to the latter are alternated with direct calls to `Chainman`'s `ProcessNewBlock`, because e.g. `net_processing.cpp` uses that.
#31564 proposes an alternative to `TestBlockValidity` which requires that `cs_main` is _not_ held. Prepare for this by not holding `cs
...
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2565696340)
I extracted the first commit into its own PR #31581. Review on the second commit is still welcome here, since I don't expect it to change. Marking draft.
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2565696340)
I extracted the first commit into its own PR #31581. Review on the second commit is still welcome here, since I don't expect it to change. Marking draft.
📝 Sjors converted_to_draft a pull request: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283)
Alternative approach to #31003, suggested in https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2451942362.
This PR introduces `waitNext()`. It waits for either the tip to update or for fees at the top of the mempool to rise sufficiently. It then returns a new template, with which the caller can rinse and repeat.
(https://github.com/bitcoin/bitcoin/pull/31283)
Alternative approach to #31003, suggested in https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2451942362.
This PR introduces `waitNext()`. It waits for either the tip to update or for fees at the top of the mempool to rise sufficiently. It then returns a new template, with which the caller can rinse and repeat.
💬 instagibbs commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2565706580)
> is there any reason you went for a multiplier?
nothing off the top of my head
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2565706580)
> is there any reason you went for a multiplier?
nothing off the top of my head
💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2565707144)
I refactored `checkblock` to take a `target` argument rather than a multiplier. In order to make this easier to implement and test, I also introduced a `gettarget` RPC and `DeriveTarget()` helper.
I then dropped the original `TestBlockValidity` and renamed `CheckNewBlock` to it. The `generateblock` and `getblocktemplate` RPC calls, as well
as `BlockAssembler::CreateNewBlock` use the new method.
If people prefer I could rewrite the git history to directly refactor `TestBlockValidity`, thou
...
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2565707144)
I refactored `checkblock` to take a `target` argument rather than a multiplier. In order to make this easier to implement and test, I also introduced a `gettarget` RPC and `DeriveTarget()` helper.
I then dropped the original `TestBlockValidity` and renamed `CheckNewBlock` to it. The `generateblock` and `getblocktemplate` RPC calls, as well
as `BlockAssembler::CreateNewBlock` use the new method.
If people prefer I could rewrite the git history to directly refactor `TestBlockValidity`, thou
...
💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#discussion_r1899676636)
This also applies to the new `DeriveTarget` method. I added the motivation for not changing `CheckProofOfWorkImpl` to the commit message in: https://github.com/bitcoin/bitcoin/pull/31564/commits/d3710b0ffe416337cd2c21ba871348b0e7b4aa9e
(https://github.com/bitcoin/bitcoin/pull/31564#discussion_r1899676636)
This also applies to the new `DeriveTarget` method. I added the motivation for not changing `CheckProofOfWorkImpl` to the commit message in: https://github.com/bitcoin/bitcoin/pull/31564/commits/d3710b0ffe416337cd2c21ba871348b0e7b4aa9e
💬 fjahr commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2565723488)
Concept ACK
The changes in the tests look good but I need a little more time with the CI stuff. Maybe the PR could have been split there to get the critical fixes in faster but I guess it's a bit late now.
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2565723488)
Concept ACK
The changes in the tests look good but I need a little more time with the CI stuff. Maybe the PR could have been split there to get the critical fixes in faster but I guess it's a bit late now.
💬 hebasto commented on pull request "test: Remove non-portable IPv6 test":
(https://github.com/bitcoin/bitcoin/pull/31580#issuecomment-2565734613)
> Is there a way to skip the test for Illumos-based platforms?
I've verified both OpenIndiana and OmniOS. The `__illumos__` macro is defined to `1` in both.
(https://github.com/bitcoin/bitcoin/pull/31580#issuecomment-2565734613)
> Is there a way to skip the test for Illumos-based platforms?
I've verified both OpenIndiana and OmniOS. The `__illumos__` macro is defined to `1` in both.
💬 achow101 commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2565759517)
> Looks like you didn't.
Oops, pushed it.
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2565759517)
> Looks like you didn't.
Oops, pushed it.
📝 achow101 opened a pull request: "[28.x] Finalize 28.1"
(https://github.com/bitcoin/bitcoin/pull/31582)
(https://github.com/bitcoin/bitcoin/pull/31582)
💬 achow101 commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2565774931)
ACK b9766c9977e58a9ebc358d9879576376e76a72b1
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2565774931)
ACK b9766c9977e58a9ebc358d9879576376e76a72b1
💬 achow101 commented on pull request "Remove unused variable assignment":
(https://github.com/bitcoin/bitcoin/pull/31497#issuecomment-2565775604)
ACK b9766c9977e58a9ebc358d9879576376e76a72b1
(https://github.com/bitcoin/bitcoin/pull/31497#issuecomment-2565775604)
ACK b9766c9977e58a9ebc358d9879576376e76a72b1
💬 achow101 commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#issuecomment-2565782475)
ACK ecaa786cc103cf7cc63ae899ec13d81a54e2fd1e
(https://github.com/bitcoin/bitcoin/pull/31531#issuecomment-2565782475)
ACK ecaa786cc103cf7cc63ae899ec13d81a54e2fd1e
📝 Sjors converted_to_draft a pull request: "Add checkblock RPC and checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31564)
## Rationale
### Verifying block templates (no PoW)
Stratum v2 and DATUM allow miners to generate their own block template. Pools may wish (or need) to verify these templates. This typically involves comparing mempools, asking miners to providing missing transactions and then reconstructing the proposed block.[^0] This is not sufficient to ensure a proposed block is actually valid. In some schemes miners could take advantage of incomplete validation[^1].
The Stratum Reference Implementa
...
(https://github.com/bitcoin/bitcoin/pull/31564)
## Rationale
### Verifying block templates (no PoW)
Stratum v2 and DATUM allow miners to generate their own block template. Pools may wish (or need) to verify these templates. This typically involves comparing mempools, asking miners to providing missing transactions and then reconstructing the proposed block.[^0] This is not sufficient to ensure a proposed block is actually valid. In some schemes miners could take advantage of incomplete validation[^1].
The Stratum Reference Implementa
...
💬 achow101 commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#issuecomment-2565786650)
ACK fa494a1d53f3f030fafe7b533d72b2200428a0fd
(https://github.com/bitcoin/bitcoin/pull/31540#issuecomment-2565786650)
ACK fa494a1d53f3f030fafe7b533d72b2200428a0fd
🚀 achow101 merged a pull request: "Remove unused variable assignment"
(https://github.com/bitcoin/bitcoin/pull/31497)
(https://github.com/bitcoin/bitcoin/pull/31497)
💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2565793245)
I might make a separate PR to consider whether to drop `AssertLockHeld(cs_main)` from the original `TestBlockValidity`.
If that ends up rejected, then I'll keep the existing `TestBlockValidity` calls as they are, i.e. drop the last two commits from this PR and leave them as a followup.
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2565793245)
I might make a separate PR to consider whether to drop `AssertLockHeld(cs_main)` from the original `TestBlockValidity`.
If that ends up rejected, then I'll keep the existing `TestBlockValidity` calls as they are, i.e. drop the last two commits from this PR and leave them as a followup.
💬 achow101 commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2565793417)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
Low powered machines have advanced quite a lot in the past decade, makes sense to increase these.
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2565793417)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
Low powered machines have advanced quite a lot in the past decade, makes sense to increase these.
💬 achow101 commented on pull request "test: report detailed msg during utf8 response decoding error":
(https://github.com/bitcoin/bitcoin/pull/31251#issuecomment-2565795242)
ACK a2c45ae5480a2ee643665d6ecaee9714a287a70e
(https://github.com/bitcoin/bitcoin/pull/31251#issuecomment-2565795242)
ACK a2c45ae5480a2ee643665d6ecaee9714a287a70e
🚀 achow101 merged a pull request: "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo"
(https://github.com/bitcoin/bitcoin/pull/31531)
(https://github.com/bitcoin/bitcoin/pull/31531)
✅ achow101 closed an issue: "Default rpcthreads value (4) is too small"
(https://github.com/bitcoin/bitcoin/issues/29386)
(https://github.com/bitcoin/bitcoin/issues/29386)