7f318e1dd0 test: Add better coverage for Autofile size() (Fabian Jahr)
b7af960eb8 refactor: Add AutoFile::size (Fabian Jahr)
ec0f75862e refactor: Modernize logging in util/asmap.cpp (Fabian Jahr)
606a251e0a tests: add unit test vectors for asmap interpreter (Pieter Wuille)
Pull request description:
This contains some commits from #28792 that can be easily reviewed and merged independently. I hope splitting this change off can make this part move a bit faster and reduce frequency of needed rebases for #28792.
The commits in order:
- Add additional unit test vectors to the asmap interpreter (written by sipa). This helps to ensure that the further refactors in #28792 don't change behavior.
- Modernizes the logging in `util/asmap.cpp`, I added this while touching the rest of the file all over anyway.
- Adds an `AutoFile::size` helper function with some additional test coverage in a separate commit
ACKs for top commit:
maflcko:
review ACK 7f318e1dd0🏀
hodlinator:
tACK 7f318e1dd0
laanwj:
Code review ACK 7f318e1dd0
Tree-SHA512: 45156b74e4bd9278a7ec24521dfdafe4dab1ba3384243c7d589ef17e16ca374ee2af7178c86b7229e80ca262dbe78c4d456d80a6ee742ec31d2ab5243dac8b57
2594d5a189 build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings (Henry Romp)
Pull request description:
Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings that are no longer needed after reordering the Guix build script to perform binary checks after installation.
This PR also removes the unused CMake maintenance targets (`check-security` and `check-symbols`) and updates the Guix security checks to include binaries in the `libexec/` directory (added in PR #31679).
ACKs for top commit:
purpleKarrot:
ACK 2594d5a189
hebasto:
ACK 2594d5a189.
Tree-SHA512: ed451a298f5aae05c177b0033b092faaa7536caeaa3d84da9b8b611e2aa905e1dd337e57aef0efd69ce6ce6ac0cf77dc57adf175079b95bf53dd96d5d0c8118b
Remove CMake settings that are no longer needed after reordering Guix build script to perform binary checks after installation.
Also removes unused CMake maintenance targets (check-security and check-symbols) and updates security checks to include libexec/ directory binaries (see PR #31679).
fa6c0bedd3 refactor: Return uint64_t from GetSerializeSize (MarcoFalke)
fad0c8680e refactor: Use uint64_t over size_t for serialized-size values (MarcoFalke)
fa4f388fc9 refactor: Use fixed size ints over (un)signed ints for serialized values (MarcoFalke)
fa01f38e53 move-only: Move CBlockFileInfo to kernel namespace (MarcoFalke)
fa2bbc9e4c refactor: [rpc] Remove cast when reporting serialized size (MarcoFalke)
fa364af89b test: Remove outdated comment (MarcoFalke)
Pull request description:
Consensus code should arrive at the same conclusion, regardless of the architecture it runs on. Using architecture-specific types such as `size_t` can lead to issues, such as the low-severity [CVE-2025-46597](https://bitcoincore.org/en/2025/10/24/disclose-cve-2025-46597/).
The CVE was already worked around, but it may be good to still fix the underlying issue.
Fixes https://github.com/bitcoin/bitcoin/issues/33709 with a few refactors to use explicit fixed-sized integer types in serialization-size related code and concluding with a refactor to return `uint64_t` from `GetSerializeSize`. The refactors should not change any behavior, because the CVE was already worked around.
ACKs for top commit:
Crypt-iQ:
crACK fa6c0bedd3
l0rinc:
ACK fa6c0bedd3
laanwj:
Code review ACK fa6c0bedd3
Tree-SHA512: f45057bd86fb46011e4cb3edf0dc607057d72ed869fd6ad636562111ae80fea233b2fc45c34b02256331028359a9c3f4fa73e9b882b225bdc089d00becd0195e
a3ac59a431 ci: Enable experimental kernel stuff in ASan task (MarcoFalke)
5b89956eeb kernel: Allow null arguments for serialized data (TheCharlatan)
Pull request description:
An empty span constructed from an empty vector may have a null data pointer depending on the implementation. Remove the BITCOINKERNEL_ARG_NONNULL requirement for these arguments and instead handle such null arguments in the implementation.
Also cherry-picked from #33845 to show that CI task passing now.
ACKs for top commit:
yuvicc:
Code review ACK a3ac59a431
maflcko:
review ACK a3ac59a431🥈
laanwj:
code review ACK a3ac59a431
Tree-SHA512: 629e463796f2f057df5be8e8981a45751c578ed0021be731c1d57fe849a539fe38b0a445914b0fc48f32f0408ad6d566984bd7f3a68797fcfdf1c6889e316a08
fa9f29a4a7 doc: Recommend latest Debian stable or Ubuntu LTS (MarcoFalke)
fa1711ee0d doc: Add GCC-12 min release notes (MarcoFalke)
faa8be75c9 ci: Enable experimental kernel stuff in G++-12 task (previous releases) (MarcoFalke)
fabce97b30 test: Remove gccbug_90348 test case (MarcoFalke)
fa3854e432 test: Remove unused fs::create_directories test (MarcoFalke)
fa9dacdbde util: [refactor] Remove unused create_directories workaround (MarcoFalke)
fa807f78ae build: Bump g++ minimum supported version to 12 (MarcoFalke)
Pull request description:
All supported operating systems that previously came with at least g++-11, also come with at least g++-12, so bumping the minimum should be fine.
For reference:
* https://packages.ubuntu.com/jammy/g++-12
* https://packages.ubuntu.com/noble/g++ (g++-13)
* https://packages.debian.org/bookworm/g++ (g++-12)
* FreeBSD Ports ship a recent GCC
* RHEL-based 8, and 9 ship with g++-14 via appstream (`dnf install gcc-toolset-14` -> `/opt/rh/gcc-toolset-14/`)
* RHEL-based 10 ships with g++ (14 by default)
* OpenSuse Leap and Tumbleweed ship with g++ 15 https://software.opensuse.org/package/gcc15-c++
Obviously, downloading pre-compiled releases or compiling previous release branches is unaffected by this change.
ACKs for top commit:
janb84:
re-ACK fa9f29a4a7
TheCharlatan:
Re-ACK fa9f29a4a7
hebasto:
ACK fa9f29a4a7.
Tree-SHA512: ce14ecf78ccfe4f221dcbc9147dcfc00c0512b23a6fcda5ba71b62b4f5d39a5139f083d035113f189bfbd396d485e1ebc626a9a16b6fa0b74fd95aed2041c841
The test case no longer detects this specific issue for GCC versions
12.1+, as explained in the
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348 thread and in this
compiler-explorer playground:
https://godbolt.org/z/Y48osrjM8
So remove the test case and update the -fstack-reuse=none cmake
docstring with the underlying affected GCC versions, and the bug URL.
The test was added in commit ddb75c2e87.
After the create_directories wrapper removal, the test is redundant with
the unit test in the upstream stdlib. Also, there is a Bitcoin Core
functional test that covers this behavior in
test/functional/feature_dirsymlinks.py
So remove this unit test.
Finally, I could not find a real system that still ships a buggy stdlib
(v11.2) in their package manager. A stand-alone test is also available
in compiler-explorer under https://godbolt.org/z/aeMKraYrT.
7a4901c902 test, refactor: Fix `-Warray-bounds` warning (Hennadii Stepanov)
faf2759c8c test: [refactor] Use reference over ptr to chainman (MarcoFalke)
Pull request description:
Just some minor test-only refactor commits to fix GCC false positive warnings, along with making the test code easier to read and understand:
* First change requested in https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2510727269
* Second change requested in commit 3b135a8fc4451c93b3ea50b3f4621e0d19f35daf
Those changes are required in a bunch of pulls touching the CI system, so merging them allows to drop them in all pulls.
ACKs for top commit:
l0rinc:
ACK 7a4901c902
hebasto:
ACK 7a4901c902, I have reviewed the code and it looks OK.
Tree-SHA512: 64dca52ec7b25078bf489e2d8b43e449f4968fbac14a09c66a60cdc75b513588403665f248368820694a6f72c4f7f465589d9306355239cffe35c38111929eff
An empty span constructed from an empty vector may have a null data
pointer depending on the implementation. Remove the
BITCOINKERNEL_ARG_NONNULL requirement for these arguments and instead
handle such null arguments in the implementation.
66978a1a95 kernel: remove btck_chain_get_tip (stickies-v)
4dd7e6dc48 kernel: remove btck_chain_get_genesis (stickies-v)
Pull request description:
Removes `btck_chain_get_genesis` and `btck_chain_get_tip`.
They are trivially replaced with `btck_chain_get_by_height` (as indicated in the updated `bitcoinkernel_wrapper.h`), so I think it makes sense to trim the interface.
For `btck_chain_get_tip`: on `master` we don't provide any guarantees that the returned block index still corresponds to the actual tip, so the extra call doesn't seem like a regression to me.
ACKs for top commit:
TheCharlatan:
ACK 66978a1a95
janb84:
ACK 66978a1a95
Tree-SHA512: f583fbb7f2e3f8f23afb57732b2cbe9e1d550bfc43c9a2619895ee30c27f5f3c5cd9e4ecb7e05b1f6ab9e11c368596ec9b733d67e06cfafb12326d88e8e4dd7d
743abbcbde refactor: inline constant return value of `BlockTreeDB::WriteBatchSync` and `BlockManager::WriteBlockIndexDB` and `BlockTreeDB::WriteFlag` (Lőrinc)
e030240e90 refactor: inline constant return value of `CDBWrapper::Erase` and `BlockTreeDB::WriteReindexing` (Lőrinc)
cdab9480e9 refactor: inline constant return value of `CDBWrapper::Write` (Lőrinc)
d1847cf5b5 refactor: inline constant return value of `TxIndex::DB::WriteTxs` (Lőrinc)
50b63a5698 refactor: inline constant return value of `CDBWrapper::WriteBatch` (Lőrinc)
Pull request description:
Related to https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2223587480
### Summary
`WriteBatch` always returns `true` - the errors are handled by throwing `dbwrapper_error` instead.
### Context
This boolean return value of the `Write` methods is confusing because it's inconsistent with `CDBWrapper::Read`, which catches exceptions and returns a boolean to indicate success/failure. It's bad that `Read` returns and `Write` throws - but it's a lot worse that `Write` advertises a return value when it actually communicates errors through exceptions.
### Solution
This PR removes the constant return values from write methods and inlines `true` at their call sites. Many upstream methods had boolean return values only because they were propagating these constants - those have been cleaned up as well.
Methods that returned a constant `true` value that now return `void`:
- `CDBWrapper::WriteBatch`, `CDBWrapper::Write`, `CDBWrapper::Erase`
- `TxIndex::DB::WriteTxs`
- `BlockTreeDB::WriteReindexing`, `BlockTreeDB::WriteBatchSync`, `BlockTreeDB::WriteFlag`
- `BlockManager::WriteBlockIndexDB`
### Note
`CCoinsView::BatchWrite` (and transitively `CCoinsViewCache::Flush` & `CCoinsViewCache::Sync`) were intentionally not changed here. While all implementations return `true`, the base `CCoinsView::BatchWrite` returns `false`. Changing this would cause `coins_view` tests to fail with:
> terminating due to uncaught exception of type std::logic_error: Not all unspent flagged entries were cleared
We can fix that in a follow-up PR.
ACKs for top commit:
achow101:
ACK 743abbcbde
janb84:
ACK 743abbcbde
TheCharlatan:
ACK 743abbcbde
sipa:
ACK 743abbcbde
Tree-SHA512: b2a550bff066216f1958d2dd9a7ef6a9949de518cc636f8ab9c670e0b7a330c1eb8c838e458a8629acb8ac980cea6616955cd84436a7b8ab9096f6d648073b1e
It is equivalent to calling btck_chain_get_by_height with the
height obtained from btck_chain_get_height. In neither case do we
provide guarantees that the returned block index still corresponds
to the actual tip.
It does not make sense to use a pointer, when a reference is more
appropriate, especially given that nullptr has been ruled out.
This is also allows to remove the CI workaround to avoid warnings:
```
C++ compiler .......................... GNU 13.0.0, /bin/x86_64-w64-mingw32-g++-posix
...
/ci_container_base/src/test/blockmanager_tests.cpp: In member function ‘void blockmanager_tests::blockmanager_scan_unlink_already_pruned_files::test_method()’:
/ci_container_base/src/test/blockmanager_tests.cpp:63:17: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
63 | const auto& chainman = Assert(m_node.chainman);
| ^~~~~~~~
In file included from /ci_container_base/src/streams.h:13,
from /ci_container_base/src/dbwrapper.h:11,
from /ci_container_base/src/node/blockstorage.h:10,
from /ci_container_base/src/test/blockmanager_tests.cpp:8:
/ci_container_base/src/util/check.h:116:49: note: the temporary was destroyed at the end of the full expression ‘inline_assertion_check<true, std::unique_ptr<ChainstateManager>&>(((blockmanager_tests::blockmanager_scan_unlink_already_pruned_files*)this)->blockmanager_tests::blockmanager_scan_unlink_already_pruned_files::<anonymous>.TestChain100Setup::<anonymous>.TestingSetup::<anonymous>.ChainTestingSetup::<anonymous>.BasicTestingSetup::m_node.node::NodeContext::chainman, std::source_location{(& *.Lsrc_loc27)}, std::basic_string_view<char>(((const char*)"m_node.chainman")))’
116 | #define Assert(val) inline_assertion_check<true>(val, std::source_location::current(), #val)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/ci_container_base/src/test/blockmanager_tests.cpp:63:28: note: in expansion of macro ‘Assert’
63 | const auto& chainman = Assert(m_node.chainman);
| ^~~~~~
cc1plus: all warnings being treated as errors
gmake[2]: Leaving directory '/ci_container_base/ci/scratch/build-x86_64-w64-mingw32'
gmake[2]: *** [src/test/CMakeFiles/test_bitcoin.dir/build.make:382: src/test/CMakeFiles/test_bitcoin.dir/blockmanager_tests.cpp.obj] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:1810: src/test/CMakeFiles/test_bitcoin.dir/all] Error 2
gmake[1]: Leaving directory '/ci_container_base/ci/scratch/build-x86_64-w64-mingw32'
gmake: *** [Makefile:146: all] Error 2
```
This false-positive warning is also fixed in later GCC versions.
See also https://godbolt.org/z/fjc6be65M
24bcad3d4d refactor: remove dead code in `CountWitnessSigOps` (Lőrinc)
Pull request description:
Found while reviewing #32840
The `nullptr` witness path was dead in normal code paths: replacing it with reference enables us deleting unreachable logic.
Code coverage proof:
https://maflcko.github.io/b-c-cov/total.coverage/src/script/interpreter.cpp.gcov.html#L2135
ACKs for top commit:
kevkevinpal:
ACK [24bcad3](24bcad3d4d)
maflcko:
review ACK 24bcad3d4d🐏
darosior:
Neat. utACK 24bcad3d4d.
stickies-v:
ACK 24bcad3d4d
Tree-SHA512: 92c87e431f06a15d8eeb02e20e9154b272c4586ddacf77c8d83783091485fb82c24ecbd711db7043a92cf6169746db24ad46a5904d694aea9d3c3aa96da725f0
6c7a34f3b0 kernel: Add Purpose section to header documentation (TheCharlatan)
7e9f00bcc1 kernel: Allowing reducing exports (TheCharlatan)
7990463b10 kernel: Add pure kernel bitcoin-chainstate (TheCharlatan)
36ec9a3ea2 Kernel: Add functions for working with outpoints (TheCharlatan)
5eec7fa96a kernel: Add block hash type and block tree utility functions to C header (TheCharlatan)
f5d5d1213c kernel: Add function to read block undo data from disk to C header (TheCharlatan)
09d0f62638 kernel: Add functions to read block from disk to C header (TheCharlatan)
a263a4caf2 kernel: Add function for copying block data to C header (TheCharlatan)
b30e15f432 kernel: Add functions for the block validation state to C header (TheCharlatan)
aa262da7bc kernel: Add validation interface to C header (TheCharlatan)
d27e27758d kernel: Add interrupt function to C header (TheCharlatan)
1976b13be9 kernel: Add import blocks function to C header (TheCharlatan)
a747ca1f51 kernel: Add chainstate load options for in-memory dbs in C header (TheCharlatan)
070e77732c kernel: Add options for reindexing in C header (TheCharlatan)
ad80abc73d kernel: Add block validation to C header (TheCharlatan)
cb1590b05e kernel: Add chainstate loading when instantiating a ChainstateManager (TheCharlatan)
e2c1bd3d71 kernel: Add chainstate manager option for setting worker threads (TheCharlatan)
65571c36a2 kernel: Add chainstate manager object to C header (TheCharlatan)
c62f657ba3 kernel: Add notifications context option to C header (TheCharlatan)
9e1bac4585 kernel: Add chain params context option to C header (TheCharlatan)
337ea860df kernel: Add kernel library context object (TheCharlatan)
28d679bad9 kernel: Add logging to kernel library C header (TheCharlatan)
2cf136dec4 kernel: Introduce initial kernel C header API (TheCharlatan)
Pull request description:
This is a first attempt at introducing a C header for the libbitcoinkernel library that may be used by external applications for interfacing with Bitcoin Core's validation logic. It currently is limited to operations on blocks. This is a conscious choice, since it already offers a lot of powerful functionality, but sits just on the cusp of still being reviewable scope-wise while giving some pointers on how the rest of the API could look like.
The current design was informed by the development of some tools using the C header:
* A re-implementation (part of this pull request) of [bitcoin-chainstate](https://github.com/bitcoin/bitcoin/blob/master/src/bitcoin-chainstate.cpp).
* A re-implementation of the python [block linearize](https://github.com/bitcoin/bitcoin/tree/master/contrib/linearize) scripts: https://github.com/TheCharlatan/bitcoin/tree/kernelLinearize
* A silent payment scanner: https://github.com/josibake/silent-payments-scanner
* An electrs index builder: https://github.com/josibake/electrs/commits/electrs-kernel-integration
* A rust bitcoin node: https://github.com/TheCharlatan/kernel-node
* A reindexer: https://github.com/TheCharlatan/bitcoin/tree/kernelApi_Reindexer
The library has also been used by other developers already:
* A historical block analysis tool: https://github.com/ismaelsadeeq/mining-analysis
* A swiftsync hints generator: https://github.com/theStack/swiftsync-hints-gen
* Fast script validation in floresta: https://github.com/vinteumorg/Floresta/pull/456
* A swiftsync node implementation: https://github.com/2140-dev/swiftsync/tree/master/node
Next to the C++ header also made available in this pull request, bindings for other languages are available here:
* Rust: https://github.com/TheCharlatan/rust-bitcoinkernel
* Python: https://github.com/stickies-v/py-bitcoinkernel
* Go: https://github.com/stringintech/go-bitcoinkernel
* Java: https://github.com/yuvicc/java-bitcoinkernel
The rust bindings include unit and fuzz tests for the API.
The header currently exposes logic for enabling the following functionality:
* Feature-parity with the now deprecated libbitcoin-consensus
* Optimized sha256 implementations that were not available to previous users of libbitcoin-consensus thanks to a static kernel context
* Full support for logging as well as control over categories and severity
* Feature parity with the existing experimental bitcoin-chainstate
* Traversing the block index as well as using block index entries for reading block and undo data.
* Running the chainstate in memory
* Reindexing (both full and chainstate-only)
* Interrupting long-running functions
The pull request introduces a new kernel-only test binary that purely relies on the kernel C header and the C++ standard library. This is intentionally done to show its capabilities without relying on other code inside the project. This may be relaxed to include some of the existing utilities, or even be merged into the existing test suite.
The complete docs for the API as well as some usage examples are hosted on [thecharlatan.ch/kernel-docs](https://thecharlatan.ch/kernel-docs/index.html). The docs are generated from the following repository (which also holds the examples): [github.com/TheCharlatan/kernel-docs](https://github.com/TheCharlatan/kernel-docs).
#### How can I review this PR?
Scrutinize the commit messages, run the tests, write your own little applications using the library, let your favorite code sanitizer loose on it, hook it up to your fuzzing infrastructure, profile the difference between the existing bitcoin-chainstate and the bitcoin-chainstate introduced here, be nitty on the documentation, police the C interface, opine on your own API design philosophy.
To get a feeling for the API, read through the tests, or one of the examples.
To configure this PR for making the shared library and the bitcoin-chainstate and test_kernel utilities available:
```
cmake -B build -DBUILD_KERNEL_LIB=ON -DBUILD_UTIL_CHAINSTATE=ON
```
Once compiled the library is part of the build artifacts that can be installed with:
```
cmake --install build
```
#### Why a C header (and not a C++ header)
* Shipping a shared library with a C++ header is hard, because of name mangling and an unstable ABI.
* Mature and well-supported tooling for integrating C exists for nearly every popular language.
* C offers a reasonably stable ABI
Also see https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2285719575.
#### What about versioning?
The header and library are still experimental and I would expect this to remain so for some time, so best not to worry about versioning yet.
#### Potential future additions
In future, the C header could be expanded to support (some of these have been roughly implemented):
* Handling transactions, block headers, coins cache, utxo set, meta data, and the mempool
* Adapters for an abstract coins store
* Adapters for an abstract block store
* Adapters for an abstract block tree store
* Allocators and buffers for more efficient memory usage
* An "[io-less](https://sans-io.readthedocs.io/how-to-sans-io.html)" interface
* Hooks for an external mempool, or external policy rules
#### Current drawbacks
* For external applications to read the block index of an existing Bitcoin Core node, Bitcoin Core needs to shut down first, since leveldb does not support reading across multiple processes. Other than migrating away from leveldb, there does not seem to be a solution for this problem. Such a migration is implemented in #32427.
* The fatal error handling through the notifications is awkward. This is partly improved through #29642.
* Handling shared pointers in the interfaces is unfortunate. They make ownership and freeing of the resources fuzzy and poison the interfaces with additional types and complexity. However, they seem to be an artifact of the current code that interfaces with the validation engine. The validation engine itself does not seem to make extensive use of these shared pointers.
* If multiple instances of the same type of objects are used, there is no mechanism for distinguishing the log messages produced by each of them. A potential solution is #30342.
* The background leveldb compaction thread may not finish in time leading to a non-clean exit. There seems to be nothing we can do about this, outside of patching leveldb.
ACKs for top commit:
alexanderwiederin:
re-ACK 6c7a34f3b0
stringintech:
re-ACK 6c7a34f
laanwj:
Code review ACK 6c7a34f3b0
ismaelsadeeq:
reACK 6c7a34f3b0👾
fanquake:
ACK 6c7a34f3b0 - soon we'll be running bitcoin (kernel)
Tree-SHA512: ffe7d4581facb7017d06da8b685b81f4b5e4840576e878bb6845595021730eab808d8f9780ed0eb0d2b57f2647c85dcb36b6325180caaac469eaf339f7258030
5d784bebaf clang-tidy: Disable `ArrayBound` check in src/ipc and src/test (Hennadii Stepanov)
5efdb0ef30 ci: Update Clang in "tidy" job (Hennadii Stepanov)
Pull request description:
This PR:
1. Updates to [IWYU 0.25](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.25), which is compatible with Clang 21.
2. Fixes new "modernize-use-default-member-init" warnings. The warning in `interpreter.cpp` is a [false positive](https://github.com/llvm/llvm-project/issues/160394), so it has been suppressed.
ACKs for top commit:
maflcko:
review ACK 5d784bebaf🎒
ryanofsky:
Code review ACK 5d784bebaf, just adding clang version comment since last review.
Tree-SHA512: a1d853675ec064170ee0f1cd16be6a900676588d4a1e7b5def8733933b140ba1a9520ec6f6a42bf7638b2ff7cf2fe4d5866d407f68b677b49d2bd68ff345f735
This introduces the transaction outpoint, input and id types. This now
allows a user to retrieve a transaction output from a prior transaction
that a transaction outpoint is pointing to by either scanning through
all available transactions, or maintaining a data structure for lookups.
This is exercised in the tests by verifying the script of every
transaction in the test chain.
Introduce btck_BlockHash as a type-safe identifier for a block. Adds
functions to retrieve block tree entries by hash or height, get block
hashes and heights from entries. access the genesis block, and check if
blocks are in the active chain.
This adds functions for reading the undo data from disk with a retrieved
block tree entry. The undo data of a block contains all the spent
script pubkeys of all the transactions in a block. For ease of
understanding the undo data is renamed to spent outputs with seperate
data structures exposed for a block's and a transaction's spent outputs.
In normal operations undo data is used during re-orgs. This data might
also be useful for building external indexes, or to scan for silent
payment transactions.
Internally the block undo data contains a vector of transaction undo
data which contains a vector of the coins consumed. The coins are all
int the order of the transaction inputs of the consuming transactions.
Each coin can be used to retrieve a transaction output and in turn a
script pubkey and amount.
This translates to the three-level hierarchy the api provides: Block
spent outputs contain transaction spent outputs, which contain
individual coins. Each coin includes the associated output, the height
of the block is contained in, and whether it is from a coinbase
transaction.
This adds functions for reading a block from disk with a retrieved block
tree entry. External services that wish to build their own index, or
analyze blocks can use this to retrieve block data.
The block tree can now be traversed from the tip backwards. This is
guaranteed to work, since the chainstate maintains an internal block
tree index in memory and every block (besides the genesis) has an
ancestor.
The user can use this function to iterate through all blocks in the
chain (starting from the tip). The tip is retrieved from a separate
`Chain` object, which allows distinguishing whether entries are
currently in the best chain. Once the block tree entry for the genesis
block is reached a nullptr is returned if the user attempts to get the
previous entry.
This adds a function for streaming bytes into a user-owned data
structure.
Use it in the tests for verifying the implementation of the validation
interface's `BlockChecked` method.
These allow for the interpretation of the data in a `BlockChecked`
validation interface callback. The validation state passed through
`BlockChecked` is the source of truth for the validity of a block (the
mode). It is
also useful to get richer information in case a block failed to
validate (the result).
This adds the infrastructure required to process validation events. For
now the external validation interface only has support for the
`BlockChecked` , `NewPoWValidBlock`, `BlockConnected`, and
`BlockDisconnected` callback. Support for the other internal
validation interface methods can be added in the future.
The validation interface follows an architecture for defining its
callbacks and ownership that is similar to the notifications.
The task runner is created internally with a context, which itself
internally creates a unique ValidationSignals object. When the user
creates a new chainstate manager the validation signals are internally
passed to the chainstate manager through the context.
A validation interface can register for validation events with a
context. Internally the passed in validation interface is registerd with
the validation signals of a context.
The callbacks block any further validation execution when they are
called. It is up to the user to either multiplex them, or use them
otherwise in a multithreaded mechanism to make processing the validation
events non-blocking.
I.e. for a synchronous mechanism, the user executes instructions
directly at the end of the callback function:
```mermaid
sequenceDiagram
participant V as Validation
participant C as Callback
V->>C: Call callback
Note over C: Process event (blocks)
C-->>V: Return
Note over V: Validation resumes
```
To avoid blocking, the user can submit the data to e.g. a worker thread
or event manager, so processing happens asynchronously:
```mermaid
sequenceDiagram
participant V as Validation
participant C as Callback
participant W as Worker Thread
V->>C: Call callback
C->>W: Submit to worker thread
C-->>V: Return immediately
Note over V: Validation continues
Note over W: Process event async
```
Add `btck_import_blocks` to import block data and rebuild indexes. The
function can either reindex all existing block files if the indexes were
previously wiped through the chainstate manager options, or import
blocks from specified file paths.
This allows a user to run the kernel without creating on-disk files for
the block tree and chainstate indexes. This is potentially useful in
scenarios where the user needs to do some ephemeral validation
operations.
One specific use case is when linearizing the blocks on disk. The block
files store blocks out of order, so a program may utilize the library
and its header to read the blocks with one chainstate manager, and then
write them back in order, and without orphans, with another chainstate
maanger. To save disk resources and if the indexes are not required once
done, it may be beneficial to keep the indexes in memory for the
chainstate manager that writes the blocks back again.
Adds options for wiping the chainstate and block tree indexes to the
chainstate manager options. In combination and once the
`*_import_blocks(...)` function is added in a later commit, this
triggers a reindex. For now, it just wipes the existing data.
The added function allows the user process and validate a given block
with the chainstate manager. The *_process_block(...) function does some
preliminary checks on the block before passing it to
`ProcessNewBlock(...)`. These are similar to the checks in the
`submitblock()` rpc.
Richer processing of the block validation result will be made available
in the following commits through the validation interface.
The commits also adds a utility for deserializing a `CBlock`
(`kernel_block_create()`) that may then be passed to the library for
processing.
The tests exercise the function for both mainnet and regtest. The
commit also adds the data of 206 regtest blocks (some blocks also
contain transactions).
This is the main driver class for anything validation related, so expose
it here.
Creating the chainstate manager options will currently also trigger the
creation of their respectively configured directories.
The chainstate manager and block manager options are consolidated into a
single object. The kernel might eventually introduce a separate block
manager object for the purposes of being a light-weight block store
reader.
The chainstate manager will associate with the context with which it was
created for the duration of its lifetime and it keeps it in memory with
a shared pointer.
The tests now also create dedicated temporary directories. This is
similar to the behaviour in the existing unit test framework.
Co-authored-by: stickies-v <stickies-v@protonmail.com>
The notifications are used for notifying on connected blocks and on
warning and fatal error conditions.
The user of the C header may define callbacks that gets passed to the
internal notification object in the
`kernel_NotificationInterfaceCallbacks` struct.
Each of the callbacks take a `user_data` argument that gets populated
from the `user_data` value in the struct. It can be used to recreate the
structure containing the callbacks on the user's side, or to give the
callbacks additional contextual information.
As a first option, add the chainparams. For now these can only be
instantiated with default values. In future they may be expanded to take
their own options for regtest and signet configurations.
This commit also introduces a unique pattern for setting the option
values when calling the `*_set(...)` function.
The context introduced here holds the objects that will be required for
running validation tasks, such as the chosen chain parameters, callbacks
for validation events, and interrupt handling. These will be used by the
chainstate manager introduced in subsequent commits.
This commit also introduces conventions for defining option objects. A
common pattern throughout the C header will be:
```
options = object_option_create();
object = object_create(options);
```
This allows for more consistent usage of a "builder pattern" for
objects where options can be configured independently from
instantiation.
Exposing logging in the kernel library allows users to follow
operations. Users of the C header can use
`kernel_logging_connection_create(...)` to pass a callback function to
Bitcoin Core's internal logger. Additionally the level and category can
be globally configured.
By default, the logger buffers messages until
`kernel_loggin_connection_create(...)` is called. If the user does not
want any logging messages, it is recommended that
`kernel_disable_logging()` is called, which permanently disables the
logging and any buffering of messages.
Co-authored-by: stringintech <stringintech@gmail.com>
As a first step, implement the equivalent of what was implemented in the
now deprecated libbitcoinconsensus header. Also add a test binary to
exercise the header and library.
Unlike the deprecated libbitcoinconsensus the kernel library can now use
the hardware-accelerated sha256 implementations thanks for its
statically-initialzed context. The functions kept around for
backwards-compatibility in the libbitcoinconsensus header are not ported
over. As a new header, it should not be burdened by previous
implementations. Also add a new error code for handling invalid flag
combinations, which would otherwise cause a crash.
The macros used in the new C header were adapted from the libsecp256k1
header.
To make use of the C header from C++ code, a C++ header is also
introduced for wrapping the C header. This makes it safer and easier to
use from C++ code.
Co-authored-by: stickies-v <stickies-v@protonmail.com>
fa4b52bd16 fuzz: refactor memcpy to std::ranges::copy to work around ubsan warn (MarcoFalke)
Pull request description:
Using std::ranges::copy from the C++ standard library has a few benefits here:
* It has the additional benefit of being a bit more type safe and document the byte cast explicitly.
* The compiler will likely optimize it to the same asm, but performance doesn't really matter here anyway.
* It has defined semantics for empty source ranges.
Fixes https://github.com/bitcoin/bitcoin/issues/33643
ACKs for top commit:
marcofleon:
tACK fa4b52bd16
dergoegge:
utACK fa4b52bd16
Tree-SHA512: 04fcf096e3cfc526e996c9313ec6e0a4d12c382fa19cb846b51564d33de2f0ef78a588fc6a936da0c76ca8bc9d9db4a824c36d99413db4f538a98239864d48f0
The values are small enough to fit in size_t, but to avoid having to
think about it, just use uint64_t consistently for all architectures.
On 64-bit systems, this refactor is a no-op. On 32-bit systems, it could
avoid bugs in the theoretical and unexpected case where a 32-bit size_t
is too small and overflows.
5fa81e239a test: add valid tx test with minimum-sized ECDSA signature (8 bytes DER-encoded) (Sebastian Falbesoner)
Pull request description:
Currently in our tests, all ECDSA signatures passing verification have sizes of 69 bytes and above (that's the DER-encoded size, i.e. counted without the sighash flag byte) [1]. This PR adds test coverage for the minimum-sized valid case of 8 bytes, by taking an interesting testnet transaction that I stumbled upon:
https://mempool.space/testnet/tx/c6c232a36395fa338da458b86ff1327395a9afc28c5d2daa4273e410089fd433
Note that this is a very obscure construction that only works because the public key used isn't contained in the locking script, but calculated and provided later at spending time (see https://bitcointalk.org/index.php?topic=1729534.msg17309060#msg17309060 for an explainer), to match the message (sighash) and picked signature. So this doesn't represent a use-case that really makes sense in practice, but it can still appear in a block (not in mempool though, due to `SCRIPT_VERIFY_CONST_SCRIPTCODE`), and having test-coverage seems useful.
Can be tested with same patch below (tests crash with the condition `>= 9`, but pass with `>= 8`).
[1] this can be verified by applying the following patch and running the tests:
```diff
diff --git a/src/pubkey.cpp b/src/pubkey.cpp
index a4ca9a170a..bee0caa603 100644
--- a/src/pubkey.cpp
+++ b/src/pubkey.cpp
@@ -288,7 +288,9 @@ bool CPubKey::Verify(const uint256 &hash, const std::vector<unsigned char>& vchS
/* libsecp256k1's ECDSA verification requires lower-S signatures, which have
* not historically been enforced in Bitcoin, so normalize them first. */
secp256k1_ecdsa_signature_normalize(secp256k1_context_static, &sig, &sig);
- return secp256k1_ecdsa_verify(secp256k1_context_static, &sig, hash.begin(), &pubkey);
+ bool ret = secp256k1_ecdsa_verify(secp256k1_context_static, &sig, hash.begin(), &pubkey);
+ if (ret) assert(vchSig.size() >= 69);
+ return ret;
}
```
ACKs for top commit:
ajtowns:
ACK 5fa81e239a lgtm
fjahr:
tACK 5fa81e239a
real-or-random:
utACK 5fa81e239a interesting case
Tree-SHA512: d1f0612fdb71c9238ca0420f574f6f246e60dbd11970b23f21d082c759a89ff98a13b12a1f6266f14f20539ec437b7ab79322082278da32984ddfee2d8893356
Using std::ranges::copy from the C++ standard library has a few benefits
here:
* It has the additional benefit of being a bit more type safe and
document the byte cast explicitly.
* The compiler will likely optimize it to the same asm, but performance
doesn't really matter here anyway.
* It works around an UB-Sanitizer bug, when the source range is empty.
Fixes https://github.com/bitcoin/bitcoin/issues/33643
fa0fa0f700 refactor: Revert "disable self-assign warning for tests" (MarcoFalke)
faed118fb3 build: Bump clang minimum supported version to 17 (MarcoFalke)
Pull request description:
Most supported operating systems ship with clang-17 (or later), so bump the minimum to that and allow new code to drop workarounds for previous clang bugs.
(Apart from dropping the small workaround, this bump allows the `ci_native_nowallet_libbitcoinkernel` CI to run on riscv64 without running into an ICE with clang-16.)
This patch will only be released in version 31.x, next year (2026).
For reference:
* https://packages.debian.org/bookworm/clang-19
* https://packages.ubuntu.com/noble/clang (clang-18)
* CentOS-like 8/9/10 ship clang-17 (and later) via Stream
* FreeBSD 12/13 ship clang-17 (and later) via packages
* OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (clang21); No idea about OpenSuse Leap
On operating systems where the clang version is not shipped by default, the user would have to use GCC, or install clang in a different way. For example:
* https://packages.debian.org/bookworm/g++ (g++-12)
* https://packages.ubuntu.com/jammy/g++ (g++-11)
* https://apt.llvm.org/, or nix, or guix, or compile clang from source, ...
*Ubuntu 22.04 LTS does not ship with clang-16 (the previous minimum required), nor with clang-17, so one of the above workarounds is needed there.*
macOS 14 is unaffected, and the previous minimum requirement of Xcode15.0 remains, see also 919e6d01e9/depends/hosts/darwin.mk (L3-L4). (Modulo compiling the fuzz tests, which requires 919e6d01e9/.github/workflows/ci.yml (L149))
ACKs for top commit:
janb84:
Concept ACK fa0fa0f700
l0rinc:
Code review ACK fa0fa0f700
hebasto:
ACK fa0fa0f700.
Tree-SHA512: 5973cec39982f80b8b43e493cde012d9d1ab75a0362300b007d155db9f871c6341e7e209e5e63f0c3ca490136b684683de270136d62cb56f6b00b0ac0331dc36
53e4951a5b Switch to ANSI Windows API in `fsbridge::fopen()` function (Hennadii Stepanov)
dbe770d921 Switch to ANSI Windows API in `Win32ErrorString()` function (Hennadii Stepanov)
06d0be4e22 Remove no longer necessary `WinCmdLineArgs` class (Hennadii Stepanov)
f366408492 cmake: Set process code page to UTF-8 on Windows (Hennadii Stepanov)
dccbb17806 Set minimum supported Windows version to 1903 (May 2019 Update) (Hennadii Stepanov)
Pull request description:
The main goal is to remove [deprecated](https://github.com/bitcoin/bitcoin/issues/32361) code (removed in C++26).
This PR employs Microsoft's modern [approach](https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page) to handling UTF-8:
> Until recently, Windows has emphasized "Unicode" -W variants over -A APIs. However, recent releases have used the ANSI code page and -A APIs as a means to introduce UTF-8 support to apps. If the ANSI code page is configured for UTF-8, then -A APIs typically operate in UTF-8. This model has the benefit of supporting existing code built with -A APIs without any code changes.
TODO:
- [x] Handle application manifests properly when building with MSVC.
- [x] Bump the minimum supported Windows version to 1903 (May 2019 Update).
- [x] Remove all remaining use cases of the deprecated `std:wstring_convert`.
- The instance in `subprocess.h` will be addressed in a follow-up PR, as additional tests are likely needed.
- The usage in `common/system.cpp` is handled in https://github.com/bitcoin/bitcoin/pull/32566.
Resolves partially https://github.com/bitcoin/bitcoin/issues/32361.
ACKs for top commit:
laanwj:
re-ACK 53e4951a5b
hodlinator:
re-ACK 53e4951a5b
davidgumberg:
untested crACK 53e4951a5b
Tree-SHA512: 0dbe9badca8b979ac2b4814fea6e4a7e53c423a1c96cb76ce894253137d3640a87631a5b22b9645e8f0c2a36a107122eb19ed8e92978c17384ffa8b9ab9993b5
