b7b249d3ad Revert "[refactor] rewrite vTxHashes as a vector of CTransactionRef" (Anthony Towns)
b9300d8d0a Revert "refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair<Wtxid, CTransactionRef>" (Anthony Towns)
df5a50e5de bench/blockencodings: add compact block reconstruction benchmark (Anthony Towns)
Pull request description:
Reconstructing compact blocks is on the hot path for block relay, so revert changes from #28391 and #29752 that made it slower. Also add a benchmark to validate reconstruction performance, and a comment giving some background as to the approach.
ACKs for top commit:
achow101:
ACK b7b249d3ad
polespinasa:
lgtm code review and tested ACK b7b249d3ad
cedwies:
code-review ACK b7b249d
davidgumberg:
crACK b7b249d3ad
instagibbs:
ACK b7b249d3ad
Tree-SHA512: dc266e7ac08281a5899fb1d8d0ad43eb4085f8ec42606833832800a568f4a43e3931f942d4dc53cf680af620b7e893e80c9fe9220f83894b4609184b1b3b3b42
493ba0f688 threading: reduce the scope of lock in getblocktemplate (kevkevinpal)
Pull request description:
This change was motivated by https://github.com/bitcoin/bitcoin/pull/32592#discussion_r2294770722
It does exactly what is said in the comment. Reducing the scope of the lock by a bit before it is needed
ACKs for top commit:
stickies-v:
re-ACK 493ba0f688
maflcko:
lgtm ACK 493ba0f688
Tree-SHA512: aa3a21ef3da6be6c0af78aa2dda61ee21c3f6d4d9c897413dba9e7d7d2a91e9e069bbc6b6684b45aadaa28d8603dd310f2c2d2e58c31bb4d864204e468fefaf1
509ffea40a ci: return to using dash in CentOS job (fanquake)
Pull request description:
`dash` is available again: https://bugzilla.redhat.com/show_bug.cgi?id=2335416.
ACKs for top commit:
maflcko:
lgtm ACK 509ffea40a
davidgumberg:
ACK 509ffea40a
janb84:
crACK 509ffea40a
Tree-SHA512: c57194b6158f6453cadb2487be232af5e37aa2234852f04a76fc80909fbfa48c7f8dd30e7be41be67dedb7ec4886930e165fdbaf746d358bb94c6ccc49d6bde6
46ca7712cb threading: remove unused template instantiations (Cory Fields)
b537a6a6db threading: remove obsolete critsect macros (Cory Fields)
0d0e0a39b4 threading: use a reverse lock rather than manual critsect macros (Cory Fields)
3ddd554d31 tests: Add Assertions in reverse_lock tests to exercise thread-safety annotations (Cory Fields)
c88b1cbf57 tests: get rid of remaining manual critsect usage (Cory Fields)
Pull request description:
Now that #32467 is merged, the only remaining usage of our old `CRITICAL_SECTION` macros (other than tests) is in `getblocktemplate()` and it can safely be replaced with a `REVERSE_LOCK`.
This PR makes that replacement, replaces the old `CRITICAL_SECTION` macro usage in tests, then deletes the macros themselves.
~While testing this a few weeks ago, I noticed that `REVERSE_LOCK` does not currently work properly with our thread-safety annotations as after the `REVERSE_LOCK` is acquired, clang still believes that the mutex is locked. #32465 fixes this problem. Without that fix, this PR would potentially allow a false-negative if code were added in the future to this chunk of `getblocktemplate` which required `cs_main` to be locked.~
~I added a test for the reverse lock here in the form of a compiler warning in `reverselock_tests.cpp` to simulate that possibility. This PR will therefore cause a new warning (and should fail a warnings-as-errors ci check) until #32465 is merged and this is rebased on top of it.~
Edit: Rebased on top of #32465, so this should now pass tests.
ACKs for top commit:
maflcko:
review ACK 46ca7712cb📌
fjahr:
Code review ACK 46ca7712cb
TheCharlatan:
ACK 46ca7712cb
furszy:
ACK 46ca7712cb
Tree-SHA512: 5e423c8539ed5ddd784f5c3657bbd63be509d54942c25149f04e3764bcdf897bebf655553338d5af7b8c4f546fc1d4dd4176c2bce6f4683e76ae4bb91ba2ec80
a602f6fb7b test: index with an unclean restart after a reorg (Martin Zumsande)
01b95ac6f4 index: don't commit state in BaseIndex::Rewind (Martin Zumsande)
Pull request description:
The committed state of an index should never be ahead of the flushed chainstate.
Otherwise, in the case of an unclean shutdown, the blocks necessary to revert
from the prematurely committed state are not be available, which would corrupt the coinstatsindex in particular.
Instead, the index state will be committed with the next ChainStateFlushed notification.
Fixes#33208
ACKs for top commit:
achow101:
ACK a602f6fb7b
stickies-v:
re-ACK a602f6fb7b
Tree-SHA512: 2559ea3fe066caf746a54ad7daac5031332f3976848e937c3dc8b35fa2ce925674115d8742458bf3703b3916f04f851c26523b6b94aeb1da651ba5a1b167a419
1c3db0ed8e doc: use new block_to_connect parameter name (stickies-v)
Pull request description:
The parameter name was previously changed from `pblock` to `block_to_connect` in 9ba1fff29e, without updating the documentation.
Addresses https://github.com/bitcoin/bitcoin/pull/33078#discussion_r2279914775.
ACKs for top commit:
purpleKarrot:
ACK 1c3db0ed8e
janb84:
ACK 1c3db0ed8e
musaHaruna:
ACK [1c3db0e](1c3db0ed8e)
Tree-SHA512: 8b12243f1d9e5586e487dd705dc5b40ff12025bb5539eb4195f7fde4df38a9fe8eb0a9570a72f9463a2420f7307358409804fcb23bb73e32ff691ac4ef5bc35a
de65c86572 doc: capnproto instruction for Alpine and Arch (Sjors Provoost)
49d1a1a363 doc: add capnproto-devel to Fedora build instruction (Sjors Provoost)
eab5518913 doc: mark bitcoin-{node,gui} installed in files.md (Sjors Provoost)
2a815d126b doc: link to capnp version bump PR (Sjors Provoost)
Pull request description:
- have `dependencies.md` link to the PR that updated the capnp version: https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2290393840
ACKs for top commit:
maflcko:
lgtm ACK de65c86572
janb84:
re ACK de65c86572
Tree-SHA512: 842d7a89ef18a8c597ca05720c41a72e67500bc93430cf2c3b074cb2f4b936f1df58b5b1e99010e1ea5c1f8a9f8875fb9c20398f915feeacecee9b2fed3cb03c
966666de9a doc: Remove wrong and redundant doxygen tag (MarcoFalke)
Pull request description:
`param@[in]` is not a valid doxygen tag. Also, no other function in this file uses the annotations, and they are redundant with the line above, so just remove them in `feerate` to fix all issues.
In other places, fix them.
ACKs for top commit:
cedwies:
ACK 966666d
janb84:
ACK 966666de9a
pablomartin4btc:
ACK 966666de9a
w0xlt:
ACK 966666de9a
Tree-SHA512: fcb6aa75c0f03b36f3caad023854ba276e0335cf47908a77006e182633b6a68f7b7d3115ef9fb97d143ca23730def05550f970265bb1fde97594ba68e724bde9
7392b8b084 miner: clamp options instead of asserting (Pieter Wuille)
Pull request description:
The `BlockAssembler::ClampOptions` function currently doesn't actually clamp most of the provided settings, but asserts that some are in range. This made sense while it was a purely internal interface.
However, with the mining IPC interface exposed in #30510, these options are now externally accessible, and it is not entirely intuitive how to set them. In particular, calling `Mining::createNewBlock` with a default-constructed `BlockCreateOptions` will right now instantly crash the bitcoin node.
This isn't a security issue, as the IPC interface is considered trusted, but it is highly unexpected I think, and rather unergonomical to have the node crash while developing against the interface.
An alternative would be exposing a way for the interface to return a failure, but I think in this case, just correcting to reasonable values is acceptable.
ACKs for top commit:
Sjors:
ACK 7392b8b084
achow101:
ACK 7392b8b084
stickies-v:
ACK 7392b8b084
ryanofsky:
Code review ACK 7392b8b084. I think ideally this would throw an exception and return a clear error to the caller, or maybe log as stickies suggested, but clamping is much better than crashing.
Tree-SHA512: 7a1e05b68edbf57beb682ee63e27666f42af6a2b70a81874d368a2cb10d107a589e0a388658c1039330b8cc9f6048479870095a9d552ca387a250ac118c1abf2
The committed state of an index should never
be ahead of the flushed chainstate. Otherwise, in the case
of an unclean shutdown, the blocks necessary to revert
from the prematurely committed state would not be
available, which would corrupt the coinstatsindex in particular.
Instead, the index state will be committed with the next
ChainStateFlushed notification.
be776a1443 wallet: Remove isminetype (Ava Chow)
009a69a616 wallet: Remove ISMINE_USED (Ava Chow)
6a7aa01574 wallet: Remove COutput::spendable and AvailableCoinsListUnspent (Ava Chow)
620abe985e interfaces, gui: Remove is_mine output parameter from getAddress (Ava Chow)
Pull request description:
The remaining isminetypes are `ISMINE_SPENDABLE` and `ISMINE_USED`.
`ISMINE_USED` is only used as a filter for caching balances and is never actually returned from `IsMine`. Since we do still want this behavior, This PR changes the caching to utilize bools and explicit members variables to account for the avoid_reuse case. This allows us to remove `ISMINE_USED`.
`ISMINE_SPENDABLE` and `ISMINE_NO` are the only things that are returned by `IsMine`. This is a bool, so it can be replaced as such.
After removing `ISMINE_USED` and `ISMINE_SPENDABLE`, we are able to remove isminetypes altogether.
ACKs for top commit:
murchandamus:
ACK be776a1443
fjahr:
reACK be776a1443
davidgumberg:
crACK be776a1443
enirox001:
re-ACK be776a1
jlest01:
reACK be776a1443
Tree-SHA512: 689759f6a6ba20a1ae988b0c3abacb15424844f29a1ec2fcb2d1ca9d87b44ae68313e8f61d6fd310281b681144f0ade67e90fcfab807e982b52ed99441d9c987
ce7d94a492 doc: add release note (Sjors Provoost)
71f29d4fa9 doc: update build and dependencies docs for IPC (Sjors Provoost)
3cbf747c32 cmake: set ENABLE_IPC by default (Sjors Provoost)
32a90e1b90 ci: use bitcoin-node for one depends job (Sjors Provoost)
b333cc14d5 ci: build one depends job without multiprocess (Sjors Provoost)
16bce9ac4c build: depends makes libmultiprocess by default (Sjors Provoost)
Pull request description:
Have depends make libmultiprocess by default. This PR causes the following behavior changes:
1. **bitcoin-node and bitcoin-gui binaries are included in releases**, due to `ENABLE_IPC` option being switched on by default in depends builds
2. `ENABLE_IPC` is also switched on by default in non-depends builds (instructions updated, #33190 does this as a standalone PR)
3. Various changes to CI: switching on `ENABLE_IPC` on in most configurations and using `bitcoin-node` binary (`bitcoin -m`) for functional tests in two of them.
4. The `bitcoin-node` and `bitcoin-gui` are added to `Maintenance.cmake` (since they're now in the release)
This PR doesn't need to do all of these things at once. However it's is simpler, avoids code churn (especially in CI), and probably less confusing to make all these changes in the same PR.
Windows is not supported yet, so `ENABLE_IPC` is off by default for it. It can be enabled after https://github.com/bitcoin/bitcoin/pull/32387.
The initial main use case for IPC is to enable experimental support for the Mining IPC interface. A working example of a Stratum v2 Template Provider client using this interface can be found here: https://github.com/Sjors/bitcoin/pull/48.
See #31756 for discussion of when this should happen. Supersedes #30975.
## Wait what, why?
The [Stratum v2 spec](https://stratumprotocol.org/specification) has been around for a few years now, mostly stable but with [ongoing activity](https://github.com/stratum-mining/sv2-spec/commits/main/) to clarify and fix more subtle issues encountered by implementers. Most of the implementation is built in Rust in a project called the Stratum Reference Implementation ([SRI](https://github.com/stratum-mining/stratum)).
[Braiins](https://demand.work) added Stratum v2 support to both their (custom) firmware and pool several years ago, though they have fallen behind on recent spec changes (update: it seems they've fixed that). Apparently [new hardware is underway](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3189623427) that supports Stratum v2 without the need for custom firmware.
[DMND pool](https://www.dmnd.work) is Stratum v2 native from the start and employs several of the SRI developers (they haven't fully launched though). The industry is rather secretive, but apparently [there is more underway](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3190601926).
What does Bitcoin Core have to do with this? Well, in Stratum v2 jargon we are the Template Provider.
Or at least, the Template Provider role needs us to make block templates. Initially back in 2023 the plan was to have Bitcoin Core implement this role entirely, see #23049. It would speak the sv2 encrypted message protocol. In fact the spec was designed around this assumption, making sure to only use cryptographic primitives already in our codebase.
I took over that effort in late 2023, but during 2024 it became quite clear there was [strong resistance](https://github.com/bitcoin/bitcoin/pull/29432#pullrequestreview-2132699185) to the idea of including all this new code, opening another network ports, etc.
At the same time there was the long running multiprocess / IPC project #10102, and the idea was born to apply that here: instead of including Stratum v2 specific stuff, we offer a general Mining interface via an IPC connection that can e.g. push out fresh block templates as fees rise above a threshold (something not possible and/or very inefficient with `getblocktemplate`). A client sidecar application then sits between the Stratum v2 world and our node.
Currently there's only one such sidecar application, maintained by me, and reusing the same codebase from the integrated approach. An attempt has been made to connect to our interface from Rust https://github.com/bitcoin-core/libmultiprocess/issues/174, which would pave the way for SRI include the Template Provider role. Plebhash below indicates he's also working on that: https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3191547244.
So with this new approach in mind, between mid 2024 until spring 2025, I introduced a new Mining interface (#30200 - #31785). At the same time Russ Ryanosky worked on more tight integration of [libmultiprocess](https://github.com/bitcoin-core/libmultiprocess), including making it a subtree in #31741. See [design/multiprocess.md](https://github.com/bitcoin/bitcoin/blob/master/doc/design/multiprocess.md).
Meanwhile I've been maintaining a fork of Bitcoin Core that includes the Template Provider, in the original integrated approach (https://github.com/Sjors/bitcoin/pull/68) as well as an IPC + sidecar variant (https://github.com/Sjors/bitcoin/pull/48). I've been shipping [regular releases](https://github.com/Sjors/bitcoin/releases), mostly after bug fixes or major rebases. The SRI team has been testing both variants, though the "official" [instruction on their web page](https://stratumprotocol.org/developers) is to stick to integrated version. Bug reports on [my repo fork](https://github.com/Sjors/bitcoin/issues?q=is%3Aissue) as well as on the [SRI repo](https://github.com/stratum-mining/stratum/issues?q=is%3Aissue%20%20label%3A%22template%20provider%22) are evidence of actual testing happening.
But as Pavlenex writes below:
> one recurring feedback I kept getting regardless of the size/type of miner is that the need to run a forked version of Bitcoin Core remains a significant barrier to adoption
This PR gets rids of that significant barrier. People can download a "pristine" version of Bitcoin Core and the only change is to start it with `bitcoin node -m -ipcconnect=unix` instead of the usual `bitcoind`.
Once that's released, I can dramatically simplify my sidecar codebase (https://github.com/Sjors/bitcoin/pull/48) by removing pretty much all Bitcoin Core code that it doesn't need. My plan is to then make that a separate repository, which should be much easier to contribute to. I can then also make (deterministically built) signed releases, while making it clear that sidecar code has nothing to do with Bitcoin Core. Perhaps later on SRI implements the same and I can stop maintaining that project.
Conceptually the situation will be a lot clearer;
- today: download forked version of `bitcoind` (or a forked version of `bitcoin-node`, plus `bitcoin-mine`), install SRI stuff
- tomorrow: download Bitcoin Core v30, install `bitcoin-mine` and SRI
- future: download Bitcoin Core v30 and SRI
<details>
<summary>
Guix hashes:
</summary>
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
6dbf29baecb1d1593087ef1306ae7c78aa160c8beb04dc016e02549ae2d6d90d guix-build-ce7d94a492e6/output/aarch64-linux-gnu/SHA256SUMS.part
4b465e5e8f9652c176aa57cfe5c289267c28c3a3c684034a9ce471b529b95275 guix-build-ce7d94a492e6/output/aarch64-linux-gnu/bitcoin-ce7d94a492e6-aarch64-linux-gnu-debug.tar.gz
85bc6fa008b83419d96443d9dcc212b46f0992387fd58fd2dda5da76536ee22c guix-build-ce7d94a492e6/output/aarch64-linux-gnu/bitcoin-ce7d94a492e6-aarch64-linux-gnu.tar.gz
5ed9ea52a8bd55361d2d9c01fbd1b25ec9970530c2776e6c1959424ba1689f52 guix-build-ce7d94a492e6/output/arm-linux-gnueabihf/SHA256SUMS.part
2e483011fac64462d3aa000b577c3c05c825506032d879e39612e096d7a6c65b guix-build-ce7d94a492e6/output/arm-linux-gnueabihf/bitcoin-ce7d94a492e6-arm-linux-gnueabihf-debug.tar.gz
7ff1e3ba54944a2be89dd7d68cb91dff6f8950de9d7b521e15dfb746965f81bd guix-build-ce7d94a492e6/output/arm-linux-gnueabihf/bitcoin-ce7d94a492e6-arm-linux-gnueabihf.tar.gz
abdf89e701b21b8c1238a8cec46aeaa55e0c3a0b88ad718636e89cde9813ca08 guix-build-ce7d94a492e6/output/arm64-apple-darwin/SHA256SUMS.part
fb55cff0296cd5474811fe5cedcf28603628729dd085eeefa007c72582459b33 guix-build-ce7d94a492e6/output/arm64-apple-darwin/bitcoin-ce7d94a492e6-arm64-apple-darwin-codesigning.tar.gz
e9aa566b1e79c467d7987b7c68fa608db788e6ddf89c4d90e524cd47b4faaf86 guix-build-ce7d94a492e6/output/arm64-apple-darwin/bitcoin-ce7d94a492e6-arm64-apple-darwin-unsigned.tar.gz
bb428fc62a1230a55f49fa3b5c7ba8d588e8fed491357f890d5a6724a38b14e9 guix-build-ce7d94a492e6/output/arm64-apple-darwin/bitcoin-ce7d94a492e6-arm64-apple-darwin-unsigned.zip
5ef4b75e94b2c8265fbc588bbb42467a84438af969fddac0ea61ced3e4113345 guix-build-ce7d94a492e6/output/dist-archive/bitcoin-ce7d94a492e6.tar.gz
4f55d56a108c8f312a502cd5dfdf0840b091861a6d502df31caf4636a203697a guix-build-ce7d94a492e6/output/powerpc64-linux-gnu/SHA256SUMS.part
66c5b1242c60e37098885a00e24efe19baee4afcd2e3d6407207523d8872f055 guix-build-ce7d94a492e6/output/powerpc64-linux-gnu/bitcoin-ce7d94a492e6-powerpc64-linux-gnu-debug.tar.gz
d9dbbee7217544eda26e77158cd82caeaef2b40fb9fc7033323e7ffe64264109 guix-build-ce7d94a492e6/output/powerpc64-linux-gnu/bitcoin-ce7d94a492e6-powerpc64-linux-gnu.tar.gz
d9b808cc5685c819abcebb4ace65f003ebc4bfedf3fca046b34de37994358782 guix-build-ce7d94a492e6/output/riscv64-linux-gnu/SHA256SUMS.part
eeeea470b1cf76515bfae14c779a3ea356d89f719d1fef1a81e8f0d6b04ab747 guix-build-ce7d94a492e6/output/riscv64-linux-gnu/bitcoin-ce7d94a492e6-riscv64-linux-gnu-debug.tar.gz
9993da4eb51618b8bd25ec88cc576496720e5589315e9eba6f3ddab25f9c3e60 guix-build-ce7d94a492e6/output/riscv64-linux-gnu/bitcoin-ce7d94a492e6-riscv64-linux-gnu.tar.gz
1b5a676580e0e79598d182f6ebbb05fb8aee2381edc3c09c042cae2600f448ab guix-build-ce7d94a492e6/output/x86_64-apple-darwin/SHA256SUMS.part
9152122d95a34d5df75305c6883c87707e7b09033fffd08e264d703ed177ef12 guix-build-ce7d94a492e6/output/x86_64-apple-darwin/bitcoin-ce7d94a492e6-x86_64-apple-darwin-codesigning.tar.gz
2793f75730dbef6bdf12b5ed7135e22ed21178abff2926dee92843837d4ab544 guix-build-ce7d94a492e6/output/x86_64-apple-darwin/bitcoin-ce7d94a492e6-x86_64-apple-darwin-unsigned.tar.gz
e89aafd7e4a330a41f470e8f0a91ea876fad7d19547b404600867413f1a8ccb7 guix-build-ce7d94a492e6/output/x86_64-apple-darwin/bitcoin-ce7d94a492e6-x86_64-apple-darwin-unsigned.zip
955b27f881927a86da3c566357ad8ca68dbe17e9652bde8c482a57ceacba92cb guix-build-ce7d94a492e6/output/x86_64-linux-gnu/SHA256SUMS.part
fd012be97bdf5c75ac12ddef21526bfdb5e17ecc77cde9c34d832194b0dc3293 guix-build-ce7d94a492e6/output/x86_64-linux-gnu/bitcoin-ce7d94a492e6-x86_64-linux-gnu-debug.tar.gz
0ecf7f80e9049369760d0e27fe6c026391ab25eae0f42336bef43e51a2621726 guix-build-ce7d94a492e6/output/x86_64-linux-gnu/bitcoin-ce7d94a492e6-x86_64-linux-gnu.tar.gz
2e8085f5fecc246d841b0bf6f28ecd0684a6cee49252fc88c1019d7586c7b7a2 guix-build-ce7d94a492e6/output/x86_64-w64-mingw32/SHA256SUMS.part
c60041e8137eda352557254c5f67fb83eeb97ecfec342ee528451bd44ee4523a guix-build-ce7d94a492e6/output/x86_64-w64-mingw32/bitcoin-ce7d94a492e6-win64-codesigning.tar.gz
b1be6b2f4de1c69c2e0e4de6dd97a4891ae9eb50d89435ef47247b5a187915a9 guix-build-ce7d94a492e6/output/x86_64-w64-mingw32/bitcoin-ce7d94a492e6-win64-debug.zip
bfe143f41a20c537145c7044aca889b28efe19072b0150042a3bd865983b3d7e guix-build-ce7d94a492e6/output/x86_64-w64-mingw32/bitcoin-ce7d94a492e6-win64-setup-unsigned.exe
94a906b83d84db7b25f7e3cfdce2a2030243f2ee6cc70b1fc088459f0b2f382d guix-build-ce7d94a492e6/output/x86_64-w64-mingw32/bitcoin-ce7d94a492e6-win64-unsigned.zip
```
</details>
ACKs for top commit:
ryanofsky:
Code review ACK ce7d94a492. This was just rebased to fix a conflict since last review.
josibake:
ACK ce7d94a492
achow101:
ACK ce7d94a492
ismaelsadeeq:
ACK ce7d94a492 and tested again on macOS by building via depends and source.
janb84:
ACK ce7d94a492
Tree-SHA512: f7ab72933854e9dfce5746cdf764944bc26eec815f97cd0aa6b54fa499c3cccb1b678861ef5c1c793de28153d46bbb6e4d1b9aa0652163b74262e2d55ec8b813
1d9f1cb4bd kernel: improve BlockChecked ownership semantics (stickies-v)
9ba1fff29e kernel: refactor: ConnectTip to pass block pointer by value (stickies-v)
Pull request description:
Subscribers to the BlockChecked validation interface event may need access to the block outside of the callback scope. Currently, this is only possible by copying the block, which makes exposing this validation interface event publicly either cumbersome or with significant copy overhead.
By using shared_ptr, we make the shared ownership explicit and allow users to safely use the block outside of the callback scope. By using a const-ref shared_ptr, no atomic reference count cost is incurred if a subscriber does not require block ownership.
For example: in #30595, this would allow us to drop the `kernel_BlockPointer` handle entirely, and generalize everything into `kernel_Block`. This PoC is implemented in https://github.com/stickies-v/bitcoin/commits/kernel/remove-blockpointer/.
---
### Performance
I have added a benchmark in a [separate branch](https://github.com/stickies-v/bitcoin/commits/2025-07/validation-interface-ownership-benched/), to ensure this change does not lead to a problematic performance regression. Since most of the overhead comes from the subscribers, I have added scenarios for `One`, `Two`, and `Ten` subscribers. From these results, it appears there is no meaningful performance difference on my machine.
When `BlockChecked()` takes a `const CBlock&` reference _(master)_:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 170.09 | 5,879,308.26 | 0.3% | 0.01 | `BlockCheckedOne`
| 1,603.95 | 623,460.10 | 0.5% | 0.01 | `BlockCheckedTen`
| 336.00 | 2,976,173.37 | 1.1% | 0.01 | `BlockCheckedTwo`
When `BlockChecked()` takes a `const std::shared_ptr<const CBlock>&` _(this PR)_:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 172.20 | 5,807,155.33 | 0.1% | 0.01 | `BlockCheckedOne`
| 1,596.79 | 626,254.52 | 0.0% | 0.01 | `BlockCheckedTen`
| 333.38 | 2,999,603.17 | 0.3% | 0.01 | `BlockCheckedTwo`
ACKs for top commit:
achow101:
ACK 1d9f1cb4bd
w0xlt:
reACK 1d9f1cb4bd
ryanofsky:
Code review ACK 1d9f1cb4bd. These all seem like simple changes that make sense
TheCharlatan:
ACK 1d9f1cb4bd
yuvicc:
Code Review ACK 1d9f1cb4bd
Tree-SHA512: 7ed0cccb7883dbb1885917ef749ab7cae5d60ee803b7e3145b2954d885e81ba8c9d5ab1edb9694ce6b308235c621094c029024eaf99f1aab1b47311c40958095
cb173b8e93 test: use local `CBlockIndex` in block read hash mismatch test to avoid data race (Lőrinc)
Pull request description:
Avoid mutating the shared active tip `CBlockIndex` in the `blockmanager_readblock_hash_mismatch` test.
Instead, construct a local `CBlockIndex` with only the required fields set, ensuring the test remains self-contained and hopefully eliminating the data race reported in https://github.com/bitcoin/bitcoin/issues/33150.
ACKs for top commit:
stickies-v:
ACK cb173b8e93
maflcko:
lgtm ACK cb173b8e93
Tree-SHA512: 790528db0659f8cc5b87ed2b316bf274af68edc6158b0ce8821baccddf8d9bc4074afcb7260e3a61d5013d24ab51cc5c31e36693b8fb5ab913a44229fd6ad36b
0df2c3c42e qt: Update `src/qt/locale/bitcoin_en.xlf` translation source file (Hennadii Stepanov)
Pull request description:
This PR follows our [Release Process](53a996f122/doc/release-process.md).
It is required for the translation string freeze, as the following PRs introduced new translatable strings:
- #31296:7b4a1350df/src/wallet/wallet.h (L945)
- #31453:7b4a1350df/src/init.cpp (L1878-L1879)
- #32896:22e689587a/src/wallet/spend.cpp (L288-L292)
**Notes for reviewers:**
1. To reproduce the diff, run:
```
cmake --preset dev-mode
cmake --build build_dev_mode --target translate
```
2. The structure of `bitconstrings.cpp` has been altered due to #33209.
3. The diff in `bitcoin_en.xlf` contains many unrelated metadata changes, so it may be easier to verify the changes in `bitcoin_en.ts`.
ACKs for top commit:
janb84:
re ACK 0df2c3c42e
Tree-SHA512: be87c096ef99ce7148d046f30427bc1480cb72b080eb8537a4eda3dfe4e856eeaa50cf6efb9a1c6af3d15e1123ec87a07101c539c066a8d4dd6afb817cd95137
5dda364c4b test: modify logging_filesize_rate_limit params (Eugene Siegel)
Pull request description:
Change `time_window` from 20s to 1h so `Reset` is not accidentally called if the test takes a while.
Change `num_lines` from 1024 to 10 since `LogRateLimiter` is parameterized and does not require logging 1MiB of data.
Fixes#33195
ACKs for top commit:
stickies-v:
re-ACK 5dda364c4b for more helpful failure logging, no other changes
janb84:
re ACK 5dda364c4b
dergoegge:
utACK 5dda364c4b
Tree-SHA512: f781402a3a47abc26314ee7cdf6c74e77da9b9d0dde44ba52e3c42f6c400830147554d7875e7d1217a2a378383e56d87e9712c84e877bb448112f703b87a52b1
Since the only remaining isminetypes are ISMINE_NO and ISMINE_SPENDABLE,
this enum is now just a bool and can be removed. IsMine is changed to
return a bool and any usage of isminetypes and isminefilters are changed
to be the remaining ISMINE_SPENDABLE case.
This isminetype is not a real isminetype as it is never returned by
IsMine. This is only used for isminefilters in one function, which can
be better represented with a bool parameter avoid_reuse.
In descriptor wallets, we consider all outputs to be spendable as we no
longer have mixed mine and watchonly in a wallet. As such,
COutput::spendable is meaningless and can be removed.
Furthermore, CoinFilterParams::only_spendable can be removed as that was
essentially checking for COutput::spendable.
Lastly, AvailableCoinsListUnspent can also be removed as the wrapper is
now only setting the feerate to std::nullopt which is trivial enough that
a dedicated wrapper is not needed.
3aef38f44b test: exercise index reorg assertion failure (furszy)
acf50233cd index: fix wrong assert of current_tip == m_best_block_index (Hao Xu)
Pull request description:
In BaseIndex::Sync(), pindex in `Rewind(pindex, pindex_next->pprev)` isn't always equal to m_best_block_index since m_best_block_index is updated every SYNC_LOCATOR_WRITE_INTERVAL seconds, during which multiple pindex update could happen. Thus the assert here is wrong.
ACKs for top commit:
achow101:
ACK 3aef38f44b
furszy:
ACK 3aef38f
mzumsande:
Code Review ACK 3aef38f44b
Tree-SHA512: 3ef9cc6dfdec10a9f95d7414c6a11aa216e4cf5974440d80ab19fc919abd2a3bd4c875718c9dc94523c33826f8582ec5a016374deb8fb2d35cd2fb7799b5c82e
