Seek compaction is causing a cascade effect in the chainstate DB, causing large parts of the database to be rewritten every ~hour.
Every periodic flush writes around 2 MiB. Since this is roughly the `write_buffer_size`, these writes regularly cause the memtable to rotate into a small L0 file. This file has a small seek budget, and with the random UTXO reads done during validation, it can get scheduled for seek compaction quickly.
That seek compaction pushes the small file down to L1. Since most UTXOs are already lower down in L4/L5, many reads that consult this file do not find the key there and continue downward. The bloom filter makes those misses cheap, but LevelDB still decrements the file's seek budget. The file then gets scheduled for another seek compaction, and the same pattern pushes it down through L2 and L3.
The expensive part happens around L3/L4. L4 has many ~32 MiB files holding the bulk of the UTXO set. When LevelDB compacts into L3, it may split the output into many smaller L3 files to limit how much L4 "grandparent" data any one output overlaps. Each of these small L3 files then gets its own small seek budget. Because chainstate keys are hash-random, each small L3 file can still have a broad key range, so many random reads consult it and quickly drain its budget. Once seek-compacted into L4, each tiny L3 file can overlap many L4 files, so compacting a few hundred KiB from L3 can require rewriting hundreds of MiB from L4. Repeating that across many small L3 files can rewrite most of the chainstate.
This is a poor fit for chainstate because UTXO keys are hash-random, the DB is large enough to have many levels, writes are relatively small and periodic, and reads are frequent. The result is that read misses trigger compactions much earlier than size pressure would, and those compactions have very high write amplification.
Disabling seek compaction may leave more files in upper levels for longer, so reads could theoretically consult more files. But Bitcoin Core enables bloom filters for all its LevelDB instances, so these misses are usually cheap in-memory filter checks rather than disk reads.
For the other DBs, the risk is much smaller. They also use bloom filters, and most are smaller and less read-heavy. With fewer levels and less random read pressure, disabling seek compaction should have little effect there.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Github-Pull: #35313
Rebased-From: 6bfdb6093bba4710d0f8313ed0113967a8b5176f
`CalculateMaximumSignedInputSize()` is passed the outpoint being sized, but that context was not used when estimating the signed input size.
Pass the outpoint through so externally selected inputs are not underestimated.
Co-authored-by: Antoine Poinsot <darosior@protonmail.com>
Github-Pull: #35228
Rebased-From: cd8d3bd937
The FreeBSD, NetBSD, and OpenBSD build guides state that ZMQ support is compiled in when the package is installed. Since WITH_ZMQ defaults to OFF, update the wording to mention the required CMake option.
Github-Pull: #35283
Rebased-From: ca93ab808c
17de6d2680 validation: correct lifetime of precomputed tx data (Antoine Poinsot)
Pull request description:
This backports #35209 to the version 30 branch.
ACKs for top commit:
achow101:
ACK 17de6d2680
Tree-SHA512: cdb1f9cb8166a5eb0c0ca0fb88583077a050e07032be8665ef1b0c446b2405ce7ce0138688507a63df4096d52e5dc57a163c239927706043d6431388ce5c16f5
This makes sure `txsdata` always outlives the Script check queue (since local
objects are destructed in reverse order of construction).
This is the root cause for a security vulnerability reported by Cory Fields in
2024 that could be exploited by crafting an invalid block to cause nodes to
read freed memory. The vulnerability was covertly fixed in commit
`492e1f09943fcb6145c21d470299305a19e17d8b`.
See security advisory for CVE-2024-52911 for more details.
Github-Pull: #35209
Rebased-From: 1ed799fb21
This effectively reverts a3cb309e7c from PR #30194.
That PR reduced the multi_index type signatures as recommended upstream, but
this is no longer supported as of boost 1.91 because it is no longer necessary.
1.91 drops support for the pre-c++11 work-arounds that bloated the type
signatures to begin with.
The upstream `BOOST_MULTI_INDEX_ENABLE_MPL_SUPPORT` define is meant to provide
compatibility with removed features, but it does not work for this case. Using
`indexed_by` directly when defining the `multi_index` (as opposed to inheriting
from it) works with all versions, and avoids the use of the back-compat define.
This is a slight regression when building against boost < 1.91 because the
bloated type signatures are reintroduced in that case, but it's not significant
enough to go to the trouble of introducing version detection and ifdefs.
Github-Pull: #35175
Rebased-From: 0bc9d354df
Apply the timeout factor inside the add_block function.
Also, force named args for the two expected strings.
Also, add trailing comma for style.
Github-Pull: #35080
Rebased-From: fa02eb87df
When a large cluster of unconfirmed transactions exceeds the limit,
calculateCombinedBumpFee() returns std::nullopt.
Previously, we continued executing and the optional value was
accessed unconditionally, leading to a std::bad_optional_access
exception.
Fix this by returning early when the returned bumped fee is null.
Note:
This is a crash for the GUI, and an uncaught exception for the RPC
bumpfee and psbtbumpfee.
Github-Pull: #34870
Rebased-From: 6072a2a6a1
The comparison is evaluated before the assignment, so total_amount
ends up holding a boolean instead of the actual amount:
total_amount = (a - b < c)
which is not what we want here. This has been working by accident.
Github-Pull: #34888
Rebased-From: 0026b330c4
CMake version 4.3 deprecated the imported target `Sqlite::Sqlite3`.
Use the preferred name `Sqlite3::Sqlite3` instead and provide an
alias for older versions of CMake. Also define the same alias when
using vcpkg.
Github-Pull: #34848
Rebased-From: 498b6eb6b5
Rather than looking for /translations, which might not exist (it doesn't
in a recent brew installed qt on macOS). i.e:
```bash
ls /opt/homebrew/opt/qtbase/share/qt
doc
libexec
metatypes
mkspecs
modules
plugins
sbom
```
Github-Pull: #34787
Rebased-From: 66d80d57b4
Calling shutil.make_archive(), does not preserve symlinks when using the
zip format, see https://github.com/python/cpython/issues/139679.
Call `zip` using subprocess instead. This code is only run when using a
macos machine, and I think it's safe to assume that zip is available, same
as codesign, and all other tools we call in this script.
Github-Pull: #34787
Rebased-From: ab137cbfe2
The `fee` field in the `getblock` RPC result (verbosity 2 and 3) may be
omitted when block undo data is not available. Marking it optional in
the `RPCResult` aligns the documented schema with the runtime behavior.
Github-Pull: #34702
Rebased-From: f580cc7e9f
Add validation for pubkeys in MuSig2 pubnonce and partial signature deserialization to prevent crashes with invalid curve points.
- Validate aggregate and participant pubkeys in PSBT MuSig2 fields
- Add comprehensive test coverage for invalid pubkey rejection
- Ensure proper error handling during PSBT deserialization
Github-Pull: #34219
Rebased-From: f51665bee7
Improve dependencies.md to document IPC dependencies better:
- Link to native_capnp.mk file not capnp.mk file so it's possible to see what
version of Cap'n Proto is being used in release binaries. This is important
since #31895 dropped the "Version Used" column and the capnp.mk file does not
include version number.
- Indicate Capn"Proto is used for IPC and link to multiprocess.md documenting
the feature.
- Link to correct PR requiring Cap'n Proto 0.7.1. Previous link was
pointing at PR which required 0.7.0.
- Mention libmultiprocess as a dependency even though it is included as a git
subtree and can be built as a cmake subproject. Libmultiprocess still needs
to be built separately when cross compiling, and is useful to build separately
when developing, and is still a depends package.
Based on 2cf352fd8e from #33623 by willcl-ark
which made similar changes in the 29.x branch.
Github-Pull: #34706
Rebased-From: b87a1c27c9
This refactor does not change any behavior, except for the integer
sanitizer warning.
Can be tested via:
UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./bld-cmake/bin/bitcoin-cli -stdinrpcpass uptime
Github-Pull: #34597
Rebased-From: fa6af85634
fbfc778b68 qt: Add patch to fix SFINAE warnings in QAnyStringView with gcc16 (Hennadii Stepanov)
8f5c205aa9 qt: add patches to fix SFINAE errors/warnings with gcc16 (Cory Fields)
b268b4b3d4 qt: add patch to fix build with gcc16 (Cory Fields)
916428f2c2 cmake: Fix `FindQt` module (Hennadii Stepanov)
Pull request description:
This PR backports commits from https://github.com/bitcoin/bitcoin/pull/34650 not related to Qt update. They fix issues when building with GCC 16 and improves Qt detecting.
ACKs for top commit:
fanquake:
ACK fbfc778b68
willcl-ark:
utACK fbfc778b68
Tree-SHA512: dd2d1b23aaad64a8eb6a279dbb7def1b29149cfdebd88e0667ebbccfd30ad9762f90dbbc6968eae148e724f0257646939360765c2fae1b5d513a11cf0c2902d8
The `find_package(Qt .. MODULE REQUIRED COMPONENTS ...)` call must treat
any missing component as a fatal error.
Github-Pull: #34650
Rebased-From: c55584575a
The error was added in commit 1ea7e45a1f,
because there was an additional confusing `AssertionError: [node 0]
Error: no RPC connection` instead of just a single `FileNotFoundError:
[Errno 2] No such file or directory`.
This is no longer needed on current master.
Also, the test is incomplete, because it was just checking bitcoind and
bitcoin-cli, not any other missing binaries.
Also, after the previous commit, it would not work in combination with
--valgrind.
Instead of trying to make it complete, and work in all combinations,
just remove it, because the already existing error will be clear in any
case.
This can be tested via:
```sh
./test/get_previous_releases.py
mv releases releases_backup
# Confirm the test is skipped due to missing releases
./bld-cmake/test/functional/wallet_migration.py
# Confirm the test fails due to missing releases
./bld-cmake/test/functional/wallet_migration.py --previous-releases
mv releases_backup releases
mv ./releases/v28.2 ./releases/v28.2_backup
# Confirm the test fails with a single FileNotFoundError
./bld-cmake/test/functional/wallet_migration.py
mv ./releases/v28.2_backup ./releases/v28.2
# Confirm the test runs and passes
./bld-cmake/test/functional/wallet_migration.py
rm ./bld-cmake/bin/bitcoind
# Confirm the test fails with a single "No such file or directory",
# testing with and without --valgrind
./bld-cmake/test/functional/wallet_migration.py
./bld-cmake/test/functional/wallet_migration.py --valgrind
```
Github-Pull: #34608
Rebased-From: fa29fb72cb
Prior to this commit, tool_bitcoin.py was failing:
```sh
$ ./bld-cmake/test/functional/tool_bitcoin.py --valgrind
TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "./test/functional/test_framework/test_framework.py", line 138, in main
self.setup()
~~~~~~~~~~^^
File "./test/functional/test_framework/test_framework.py", line 269, in setup
self.setup_network()
~~~~~~~~~~~~~~~~~~^^
File "./test/functional/tool_bitcoin.py", line 38, in setup_network
assert all(node.args[:len(node_argv)] == node_argv for node in self.nodes)
~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError
```
This commit fixes this issue by running `bitcoin` under valgrind. Also,
it comes with other improvements:
* Drop the outdated valgrind 3.14 requirement, because there is no
distro that ships a version that old anymore.
* Drop the VALGRIND_SUPPRESSIONS_FILE env var handling, because it was
presumably never used since it was introduced. Also, the use-case
seems limited.
Review note:
The set_cmd_args was ignoring the --valgrind test option.
In theory, this could be fixed by refactoring Binaries::node_argv() to
be used here. However, for now, just re-implement the node_argv logic in
set_cmd_args to prepend the valgrind cmd.
Github-Pull: #34608
Rebased-From: fa03fbf7e3
